-
Notifications
You must be signed in to change notification settings - Fork 158
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remember the UI that was last selected via the application switcher #6173
Conversation
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. |
@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? |
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. |
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
Agree. Users do not need to choose other apps. |
I would recommend to go with the 1st approach "remember":
|
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:
|
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. |
180f8c5
to
37b5650
Compare
async clickApp(appEntry) { | ||
// @TODO use id or similar | ||
if (appEntry.iconMaterial === 'switch_ui') { | ||
await this.setClassicUIDefault() | ||
} | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, this is a bit odd, but we need to have a reserved id or so.
SonarCloud Quality Gate failed. |
@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. |
key="apps-menu-external-link" | ||
:target="n.target" | ||
:href="n.url" | ||
@click="clickApp(n)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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. 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Set the default app to oc10
- 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...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
In general I like it. 🤩 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 |
b30572c
to
d57ec53
Compare
@pascalwengerter AFAIK, this was decided by @pmaier1 |
SonarCloud Quality Gate failed. |
Results for oCISFiles3 https://drone.owncloud.com/owncloud/web/22530/57/1
|
Results for oCISSharingPublic1 https://drone.owncloud.com/owncloud/web/22530/67/1
|
@pmaier1 @kulmann @JammingBen how to proceed with this ideas? FYI @mmattel |
Let's move on together with owncloud/core#39600. |
Moving further after oC 10.11.0 will be released |
10.11 will carry the feature to set the default application on a per user basis. Can we move on here? |
d57ec53
to
74c80c7
Compare
Results for oCISSharingBasic https://drone.owncloud.com/owncloud/web/28183/54/1 💥 The oCISSharingBasic tests pipeline failed. The build has been cancelled. |
packages/web-runtime/src/components/Topbar/ApplicationsMenu.vue
Outdated
Show resolved
Hide resolved
packages/web-runtime/src/components/Topbar/ApplicationsMenu.vue
Outdated
Show resolved
Hide resolved
packages/web-runtime/src/components/Topbar/ApplicationsMenu.vue
Outdated
Show resolved
Hide resolved
packages/web-runtime/src/components/Topbar/ApplicationsMenu.vue
Outdated
Show resolved
Hide resolved
packages/web-runtime/src/components/Topbar/ApplicationsMenu.vue
Outdated
Show resolved
Hide resolved
SonarCloud Quality Gate failed. |
* Automated changelog update [skip ci] * Revert "[full-ci] Merge master into experimental (#7460)" This reverts commit 83bc21c. * Automated changelog update [skip ci] * use latest selenium images * update docs * pin selenium to 104.0-20220812 * refactor: use public link context composable in file details * fix: close app redirect to '/' if no context route name given * Hide share actions for space viewers/editors * Fix unit tests * Call update resource on file version restore (#7469) * Call update resource on file version restore * Automated changelog update [skip ci] * Bugfix: Dragging a file causes no selection (#7473) * Fix drag & drop without selection * Add changelog * Automated changelog update [skip ci] * fix: always return within detected context If we don't return within the detected context other contexts would get resolve attempts as well. * Avoid NavigationDuplicated error in console (#7472) * Improve users table layout on small screens (#7476) * Automated changelog update [skip ci] * Fix: Sidebar cripples file name which is not visible (#7475) * Fix: Sidebar cripples file name which is not visible * Automated changelog update [skip ci] * Automated changelog update [skip ci] * Bump ocis commit id for tests * Rename method to isShareModifiable * Automated changelog update [skip ci] * added test for spaces publiclink story * User management -> app template component -> add test (#7461) Add tests * Automated changelog update [skip ci] * Thumbnail service redesign (#7474) * Prevent unnecessary PROPFIND request during upload * addressed reviwe * Automated changelog update [skip ci] * [full-ci] Make ui small again (#7363) * Automated changelog update [skip ci] * ci: skip unstable link expiry test * Fix missing space image in sidebar * Fix 'Shared via'-indicator for links (#7479) * Automated changelog update [skip ci] * Automated changelog update [skip ci] * Fix line break for 'Paste here'-button * Automated changelog update [skip ci] * fix: use alias link role capability correctly * [tx] updated from transifex * Used cache bucket for short term caching * Automated changelog update [skip ci] * Right sidebar to views (#7501) * [tx] updated from transifex * Fix right sidebar content on small screens * add change log * Re-add button for resetting file selection, remove size info component * Add resource name to the WebDAV properties (#7485) * Automated changelog update [skip ci] * Automated changelog update [skip ci] * Implement an action for clearing the current selection * Fix file name for shared files in text editor * Automated changelog update [skip ci] * Add word-break (#7482) * fix: changelog item remove blank line * Automated changelog update [skip ci] * fix: preview loading in share jail * [tx] updated from transifex * Apply responsive measures to more top bar actions * Automated changelog update [skip ci] * Add a resize observer to conditionally show/hide tooltips * Prevent context menu labels from hiding * Fix sidebar loading for the current folder * Automated changelog update [skip ci] * Fix glitchy left sidebar when switching apps * Stuck after session expired (#7491) Co-authored-by: gitstart <gitstart@users.noreply.github.com> * Automated changelog update [skip ci] * [tx] updated from transifex * Bump commit id for tests * update expected to fail file * Automated changelog update [skip ci] * Automated changelog update [skip ci] * Change quota handling (#7522) Change quota handling * Automated changelog update [skip ci] * Refactor code for upload nightly fail * Fix console error, while enter 0 (#7530) * Fix console error, while entering 0 * Automated changelog update [skip ci] * [full-ci] Bugfix: Paste action (keyboard) not working in project spaces (#7514) * Extend keyboard actions focus * Add global paste shortcut * Add changelog, linting, snapshots * Make ctrl+c, +v, +x global * Linting * Refactor function names * Update snapshot * Bind files-view instead of files * Update snapshots * Automated changelog update [skip ci] * [tx] updated from transifex * move to async script execution command * [tests-only] e2etest for spaces publiclink- Part2 (#7484) * added test for manager member * added test for resource link * added test for user carol * addressed review * address reviews Co-authored-by: Swikriti Tripathi <swikriti808@gmail.com> * [full-ci][tests-only] use oCIS from the cache used from multiple repos (#7523) * [tx] updated from transifex * add step sharing with group lint fix fix run reshare test in oc10 * Adds WEB_UI_CONFIG path in missing drone pipeline * Add default WEB_UI_CONFIG env * [tx] updated from transifex * resolve file duplicate name on creating new file (#7555) resolve unique name on creating new file * Automated changelog update [skip ci] * Only update changed data (#7538) Only patch user data if changes are detected * Automated changelog update [skip ci] * The acceptance tests pipelines now only depend on the unit tests pipelines Signed-off-by: Kiran Parajuli <kiranparajuli589@gmail.com> [not-fore-merge] intenionally fail a e2e test to demonstrate the full-ci behaviour Signed-off-by: Kiran Parajuli <kiranparajuli589@gmail.com> remove intentionally added failure Signed-off-by: Kiran Parajuli <kiranparajuli589@gmail.com> Adress reviews Signed-off-by: Kiran Parajuli <kiranparajuli589@gmail.com> * generate pipelines using matrices * fix building github comment step * move sidebar state into views * fix: add top margin to right sidebar nav section * test: unit tests for useSideBar * Add hover effect for left sidebar * refactor: rename "sidebar" folder to "sideBar" * Remove transition delay on sidebar text * feat: don't open right sidebar on scrollTo * refactor: rename more sidebar folders to sideBar * Automated changelog update [skip ci] * Adjust spacing of the files list options menu * fix flaky * go directly to share panel * [full-ci] Upgrade uppy and its packages to v3.0.0 (#7515) * Automated changelog update [skip ci] * [tx] updated from transifex * Automated changelog update [skip ci] * Spaces fixes (#7576) * fix: don't apply hover and focus nav item style to active item (#7575) * Automated changelog update [skip ci] * Bump ocis commit id to latest (#7577) * [tx] updated from transifex * Automated changelog update [skip ci] * Load groups via graph api (#7568) * Load groups via graph api * Decrease linter errors, keep commented tests warnings (#7581) * Decrease linter errors, keep commented tests warnings * [tx] updated from transifex * Replace build-web-integeration with cache * Remove restoreyarn * Enhancement: Remove clickOutside directive (#7584) * Enhancement: Remove clickOutside directive * Update changelog * remove clickOutside * Automated changelog update [skip ci] * Common search improvements (#7586) * Automated changelog update [skip ci] * Fix links capabilities checks (#7595) * Automated changelog update [skip ci] * Fix: merge shares with group and group member Sorts the list of incoming shares by path and allows merging of share with group and group member into one share through listing them next to each other * Reduce pagination options * changelog item * lint * Automated changelog update [skip ci] * [tx] updated from transifex * chore: simplify mime type checking This removes the dependency to guzzle. * fix: allow fonts path in oc10 web app * Automated changelog update [skip ci] * chore: update ODS to v14.0.0-alpha.17 * Automated changelog update [skip ci] * [full-ci] Add search support for shares (#7560) * Automated changelog update [skip ci] * Change save dialog placement (#7609) * Change save dialog placement * Remember the UI that was last selected via the application switcher (#6173) * Automated changelog update [skip ci] * Update yarn.lock file * Compare Save Dialog, simplify * Bump OCIS_COMMITID * fix: set up translations for web-client and web-pkg * [tx] updated from transifex * fix: use short language codes * Bump ocis commit id to latest * Automated changelog update [skip ci] * Automated changelog update [skip ci] * Bump ODS to 14.0.0-alpha.18 (#7626) * Bump ODS to 14.0.0-alpha.18 * Update changelog * test: update unit test snapshots Co-authored-by: Benedikt Kulmann <benedikt@kulmann.biz> * Automated changelog update [skip ci] * Prepare v5.7.0-rc.1 * Prepare v5.7.0-rc.2 * Prepare v5.7.0-rc.3 * Prepare v5.7.0-rc.4 * Adjust expected failures after ocis bump Applying same changes as in the ocis repo that came with a reva update. * Prepare v5.7.0-rc.5 * Prepare v5.7.0-rc.6 * Prepare v5.7.0-rc.7 * Prepare v5.7.0-rc.8 * Prepare v5.7.0-rc.9 * Prepare v5.7.0-rc.10 * Prepare v5.7.0-rc.11 * fix: allow empty sortBy and sortDir in SharedWithMeSection * Prepare v5.7.0-rc.12 * Prepare v5.7.0-rc.13 * fix: load client and pkg translations in runtime * Prepare v5.7.0 final * Automated changelog update [skip ci] * Fix sidebar toggle icon * Add e2e tests for searching in personal (#7583) * Add language param (#7631) * Add language param * Add changelog * Fix linting * Automated changelog update [skip ci] * Automated changelog update [skip ci] * Include `x-oc-mtime` header in upload requests (#7630) * Automated changelog update [skip ci] * [full-ci] Fix sharesTree loading (#7580) * Fix sharesTree loading * Fix parent share fetching in sidebar * Remove logs * Minor adjustment * Add changelog item * Fix loading of share indicators * Move sharesTree loading to the sidebar component * Simplify code * Fix unit tests * Make share indicators in details panel reactive again * Fix space member loading * Fix sidebar panel opening * Remove unused method * Fix e2e tests * Apply small changes according to code review * Fix e2e tests * Import isEqual directly * Automated changelog update [skip ci] * [full-ci] Resolve upload existing folder conflict dialog (#7504) * It aint much but it kinda works * Implement "keep both" * Add changelog * remove dev leftover * Fix folder name * Add isFolder * Make file conflict dialog work * Linting * Fix folder keep both * Check for folder to already exist * remove dev leftover * Address PR issues * Use store * Provide existing files with function parameter * Add type to interface * Refactor resolve file & folder conflicts * Refactor conflict dialog * Bugfix, remove dev leftover * Simplify conflict-array structure * Fix folder upload * Add merge to folders * Ignore existing folder errors for now * Make Merge reappear if "do for all" ticked * Address PR issues * Add unittests * Add more unittests * Fix e2e upload version * Fix file overwrite acceptance tests * Address PR issues Co-authored-by: Jannik Stehle <jannik.stehle@gmail.com> * Automated changelog update [skip ci] * [tx] updated from transifex * Add step for removing manager (#7637) * Bump ocis commit id for tests * Fix 'Private link'-button alignment * Automated changelog update [skip ci] * [full-ci] Search improvements (#7599) * Fix mtime headers for tus requests * Automated changelog update [skip ci] * [full-ci] Enhancement: Make arrow-key navigation global (#7569) * Add forbidden ids * Remove old code * Linting, Unittests * Add CustomKeyBindings directive * Use data-attribute instead of directive * Custom keyboard actions searchbar * Add custom keybindings to FileLinks, FileShares, SpaceMembers * Add changelog * Update snapshots * Make all keybinds global * Update changelog * Linting, Cleanup KeyboardActions * Add keycode lib * Fix custom key bindings errors * Linting, Snapshots * DEV * Linting * Sanity test * Sanity test 2 * Sanity test 3 * Make spacebar shortcut local * Update snapshots * Address PR issues, remove dev leftover * Fix Linting * Address PR issues * Automated changelog update [skip ci] * [tx] updated from transifex * update proxy config of the deployment example * [full-ci] Migrate deny-acl UI code from CERNbox (#7191) * Migrate deny-acl UI code from CERNbox Co-authored-by: Florian Schade <f.schade@icloud.com> Co-authored-by: Michael Barz <mbarz@owncloud.com> Co-authored-by: Benedikt Kulmann <benedikt@kulmann.biz> * Automated changelog update [skip ci] * [tx] updated from transifex * Fix merge error Signed-off-by: Kiran Parajuli <kiranparajuli589@gmail.com> Co-authored-by: Florian Schade <f.schade@icloud.com> Co-authored-by: Saw-jan <saw.jan.grg3e@gmail.com> Co-authored-by: Benedikt Kulmann <benedikt@kulmann.biz> Co-authored-by: Jannik Stehle <jannik.stehle@gmail.com> Co-authored-by: Paul Neubauer <paulneubauer@live.de> Co-authored-by: Jannik Stehle <50302941+JammingBen@users.noreply.github.com> Co-authored-by: Swikriti Tripathi <swikriti808@gmail.com> Co-authored-by: sushmita56 <poudelsushmita878@gmail.com> Co-authored-by: Phil Davis <phil@jankaritech.com> Co-authored-by: gitstart <gitstart@users.noreply.github.com> Co-authored-by: Sushmita Poudel <66173400+sushmita56@users.noreply.github.com> Co-authored-by: ownClouders <devops@owncloud.com> Co-authored-by: Prarup Gurung <grgprarup@gmail.com> Co-authored-by: Artur Neumann <artur@jankaritech.com> Co-authored-by: Dominik Schmidt <dschmidt@owncloud.com> Co-authored-by: GitStart <1501599+gitstart@users.noreply.github.com> Co-authored-by: sagargurung1001@gmail.com <sagargurung1001@gmail.com> Co-authored-by: PKiran <39373750+kiranparajuli589@users.noreply.github.com> Co-authored-by: Viktor Scharf <scharf.vi@gmail.com> Co-authored-by: amrita <anukanxi05@gmail.com> Co-authored-by: Kiran Parajuli <kiranparajuli589@gmail.com> Co-authored-by: Prarup Gurung <grgprarup@users.noreply.github.com> Co-authored-by: Diogo Castro <diogo.castro@cern.ch> Co-authored-by: Elizaveta Ragozina <elizaveta.ragozina@cern.ch> Co-authored-by: elizavetaRa <svillyfly@mail.ru> Co-authored-by: Pascal Wengerter <pascal@wengerter.info> Co-authored-by: Swikriti Tripathi <41103328+SwikritiT@users.noreply.github.com> Co-authored-by: Willy Kloucek <wkloucek@owncloud.com> Co-authored-by: David Christofas <dchristofas@owncloud.com> Co-authored-by: Michael Barz <mbarz@owncloud.com>
Description
This needs owncloud/core#39600 to work.
Related Issue
Types of changes
Checklist: