Remember the UI that was last selected via the application switcher

[JammingBen]

Description

This needs owncloud/core#39600 to work.

Related Issue

• https://github.com/owncloud/enterprise/issues/4694
• Closes [oc10] Let users choose if they want to use the new or classic design by default #5610

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Technical debt
• Tests

Checklist:

• Code changes
• Unit tests added
• Acceptance tests added
• Documentation ticket raised:

[update-docs]

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

[JammingBen]

@pmaier1 This is the follow up for https://github.com/owncloud/enterprise/issues/4694. Please have a look at the top post, especially the screenshot.

What do you think about the placement of the setting and the message? We kinda need a "click here" to redirect the user to the files page for it to take effect as the settings page is not yet available in Web. Does it require an additional explanation beneath the checkbox?

[AlexAndBear]

Additionally, to say, we could also let the user set the default app by providing a select and let the user choose between several apps in the core, but from our point of view, this is not quite user-friendly.

[pmaier1]

Thanks, looks good so far. Still, this approach might be a little hard to find for the average user. Maybe there are some other ideas? @tbsbdr

Additionally, to say, we could also let the user set the default app by providing a select and let the user choose between several apps in the core, but from our point of view, this is not quite user-friendly.

Agree. Users do not need to choose other apps.

[tbsbdr]

I would recommend to go with the 1st approach "remember":

1. Remember
   UX wise I guess most user would expect that the UI - Classic or Web UI - just stays the way they chose it via the app switcher. So the system should remember the last choice i took via the app switcher regarding the design.
2. Set explicitly
   An additional option in the settings would be nice-to-have for those users, who expect to change the appearance from within the settings. But I think that's more of a corner case.

[JammingBen]

I agree that 1. is the cleanest approach UX wise, although we'd need to check for the technical details. It makes this feature more appealing and easy to use. 2 issues I still struggle with though:

• There is no "Switch back to the old UI" toggle once the new UI has been activated. IMO that would be nice to have with this approach. -> this already works
• The settings are always classic UI (currently). Once I am in the settings and want to go back to the files list, I usually go via the app switcher and select "Files". Now I still see the classic UI, even if I've selected the new UI as default. That might be confusing for users... I guess it's ok for now as we will have the settings in the new UI eventually.

[tbsbdr]

As just discussed:

The switch back and forth should work via the app switcher for now.

Let's focus on the "Remember" approach. With that, explicit settings are not necessary for a first version.
cc @mcarioscio-ownCloud (fyi for sciebo)

[AlexAndBear]

Well, this is a bit odd, but we need to have a reserved id or so.

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
1 Security Hotspot
1 Code Smell

7.7% Coverage
0.0% Duplication

[JammingBen]

@kulmann What do you think of the technical solution, especially the web-part? Also I'm not sure about adding tests. I think due to SonarCloud we need at least unit or integration tests.

[pascalwengerter]

Not sure if having a click-handler on an <a> tag is optimal here. You could go for the OcButton component and define the type there maybe?

[JammingBen]

Good point! However, this links to an external page, so I'm not sure if we can use a button here (in regards to a11y).

Buttons should be used when triggering some kind of backend logic, which is definitely true here. At the same time, buttons shouldn't be used when linking to external pages, which is also true here.

Or do you mean using type="a" for the button? But it would be rendered as an a element anyway, right?

[pascalwengerter]

I think we'll have to render a second type of list items here, depending on whether they're supposed to be routing (<a>) or trigger an async action (<button>)

<li v-for="item of menuItems">
  <a v-if="item.target">...</a>
  <button v-else @click="item.clickHandler()">...</a>
</lI>

or go for a more sophisticated solution, but this will most likely get a bit messy with (optional) attributes and click handlers

<li v-for="item of menuItems">
  <oc-button :type="menuItemType(item)">...</oc-button>
</lI>

...
  computed: {
    return item.target ? 'a' : ''
  }

What do you think?

[kulmann]

What's the reason for adding the click handler to the link anyway? I'd expect a dedicated item in the user menu (not in the app switcher) for going to the default settings in oc10. Not an implicit magical behaviour when going back to the classic UI. 😉

[kulmann]

Difficulty with that is, that we don't know if web runs from within an oc10 app at all. So we don't exactly know when to show that menu item...

[AlexAndBear]

@kulmann

1. Set the default app to oc10
2. forward to oc10

You can see the handler as an interceptor

Difficulty with that is, that we don't know if web runs from within an oc10 app at all. So we don't exactly know when to show that menu item...

??? Don't get this point, there is a condition in the handler which only reacts to the switch_ui icon type...

[AlexAndBear]

@pascalwengerter

I think we'll have to render a second type of list items here, depending on whether they're supposed to be routing () or trigger an async action ()

It is still doing both, I don't think we need to change something.

[kulmann]

@kulmann What do you think of the technical solution, especially the web-part? Also I'm not sure about adding tests. I think due to SonarCloud we need at least unit or integration tests.

In general I like it. 🤩 Just the part with the implicit default handling when switching back to the classic UI is a bit irritating.

[pascalwengerter]

@kulmann What do you think of the technical solution, especially the web-part? Also I'm not sure about adding tests. I think due to SonarCloud we need at least unit or integration tests.

In general I like it. star_struck Just the part with the implicit default handling when switching back to the classic UI is a bit irritating.

I think this needs some clarification whether the PR is about remembering the UI that was last used (currently the case code-wise) as opposed to saving the user's preference (as described in some ticket IIRC), no?

Also, will need to get rebased after #6142 has been merged

[AlexAndBear]

@pascalwengerter AFAIK, this was decided by @pmaier1

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

7.1% Coverage
0.0% Duplication

[ownclouders]

Results for oCISFiles3 https://drone.owncloud.com/owncloud/web/22530/57/1
The following scenarios passed on retry:

• webUIRenameFiles/renameFiles.feature:62

[ownclouders]

Results for oCISSharingPublic1 https://drone.owncloud.com/owncloud/web/22530/67/1
The following scenarios passed on retry:

• webUISharingPublicBasic/publicLinkCreate.feature:190

[AlexAndBear]

@pmaier1 @kulmann @JammingBen how to proceed with this ideas?

FYI @mmattel

[pmaier1]

Let's move on together with owncloud/core#39600.

[AlexAndBear]

Moving further after oC 10.11.0 will be released

[pmaier1]

10.11 will carry the feature to set the default application on a per user basis. Can we move on here?

[ownclouders]

Results for oCISSharingBasic https://drone.owncloud.com/owncloud/web/28183/54/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUISharingAcceptShares-acceptShares_feature-L228.png

💥 The oCISSharingBasic tests pipeline failed. The build has been cancelled.

[sonarcloud]

SonarCloud Quality Gate failed.   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

25.0% Coverage
0.0% Duplication