Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Separate app switcher from app navigation sidebar #2669

Merged
merged 1 commit into from
Dec 18, 2019
Merged

Separate app switcher from app navigation sidebar #2669

merged 1 commit into from
Dec 18, 2019

Conversation

PVince81
Copy link
Contributor

@PVince81 PVince81 commented Dec 10, 2019 •
edited
Loading

Description

  • App switcher now appears on the top right, items need to be configured in config.json's "menu/items" section
  • Account + Logout now appear in the new User menu on the top right under the user's avatar
  • Left sidebar now only contains the current apps' menu items

Related Issue

Fixes #2650

Motivation and Context

See ticket

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

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:

Open tasks:

  • Fix styles, it currently looks ugly
  • Align config format for application menu with spec: Separate app switcher from app specific navigation #2650 (comment)
  • Proper icon for the app switcher (need icon in design system first)
  • Solve l10n for menu entries
  • Properly wire up the "/account" route
  • Issues when toggling multiple dropdowns / sidebar. Need proper exclusion
  • Clicking sidebar entry must close it
  • Optional: Filter sidebar entries to only contain entries from the currently visible app
  • Optional: better way for passing in attributes from outside like the current user ?! Or access global store directly ? We need to keep encapsulation to be able to use those as web components later on
  • top left menu label should state the name of the current app instead of just "Menu". keep the label there even if there is no menu
  • See other TODOs

@PVince81 PVince81 self-assigned this Dec 10, 2019
This was referenced Dec 10, 2019
Closed
Closed
Closed
@ownclouders
Copy link
Contributor

💥 Acceptance tests Notifications failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-162800-581.png
20191210-162833-574.png
20191210-162904-961.png
20191210-162956-759.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Login failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-162925-867.png
20191210-163001-801.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingNotifications failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-162910-778.png
20191210-162950-413.png
20191210-163026-702.png
20191210-163102-627.png
20191210-163138-722.png
20191210-163215-656.png
20191210-163251-860.png
20191210-163348-002.png
20191210-163424-899.png
20191210-163501-286.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Upload failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-163637-685.png
20191210-163735-291.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests RestrictSharing failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-163316-120.png
20191210-163426-032.png
20191210-163752-098.png
20191210-163812-489.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Trashbin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-163334-304.png
20191210-163434-574.png
20191210-164455-130.png
20191210-164611-858.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingPermissionsUsers failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-164132-926.png
20191210-164244-962.png
20191210-164416-572.png
20191210-164527-270.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests RenameFolders failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-163706-464.png
20191210-163806-875.png
20191210-163845-704.png
20191210-163946-825.png
20191210-164026-304.png
20191210-164144-108.png
20191210-164413-844.png
20191210-164511-127.png
20191210-164611-166.png
20191210-164653-991.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests RenameFiles failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-163446-706.png
20191210-163542-701.png
20191210-163638-998.png
20191210-163716-983.png
20191210-163814-588.png
20191210-163910-692.png
20191210-164408-544.png
20191210-164447-451.png
20191210-164803-347.png
20191210-164900-902.png

DeepDiver1975
DeepDiver1975 requested changes Dec 10, 2019
View reviewed changes
config.json.sample Outdated Show resolved Hide resolved
src/components/ApplicationsMenu.vue Outdated Show resolved Hide resolved
src/components/ApplicationsMenu.vue Outdated Show resolved Hide resolved
src/components/Top-Bar.vue Outdated
</oc-navbar-item>
</oc-navbar>
<div>
<message-bar />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep the message bar out of the top bar - just for the sake of keeping it small and simple.
Same for the notifications bell

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've now removed the message-bar as it was anyway duplicated.

I'll leave notifications, etc here for now and address its extraction when I get back to the "top bar as web component" topic

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added TODO in my web component PR: #2620 (comment)

src/components/UserMenu.vue Outdated Show resolved Hide resolved
src/components/UserMenu.vue Outdated Show resolved Hide resolved
src/components/UserMenu.vue Outdated Show resolved Hide resolved
src/components/UserMenu.vue Outdated Show resolved Hide resolved
src/components/UserMenu.vue Outdated Show resolved Hide resolved
@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingInternalUsers failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-164332-004.png
20191210-164627-609.png
20191210-164703-791.png
20191210-164737-591.png
20191210-164827-303.png
20191210-164827-525.png
20191210-164827-735.png
20191210-164901-813.png
20191210-164902-020.png
20191210-164902-363.png
20191210-164958-453.png
20191210-165054-855.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Resharing failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-165431-214.png
20191210-165548-320.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingPublic failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-163818-528.png
20191210-163831-535.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests XGAPortrait failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-164420-111.png
20191210-164520-565.png
20191210-164714-919.png
20191210-164735-132.png
20191210-165332-588.png
20191210-165406-270.png
20191210-165440-417.png
20191210-165514-575.png
20191210-165547-319.png
20191210-165619-599.png
20191210-165652-935.png
20191210-165730-816.png
20191210-170742-990.png
20191210-170839-362.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests iPhone failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-164318-181.png
20191210-164431-938.png
20191210-164639-542.png
20191210-164659-673.png
20191210-165517-939.png
20191210-165555-879.png
20191210-165655-085.png
20191210-165733-325.png
20191210-165810-184.png
20191210-165849-433.png
20191210-165928-417.png
20191210-170007-414.png
20191210-170919-535.png
20191210-170959-183.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Files failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7079/

20191210-163218-077.png
20191210-163258-815.png
20191210-163340-915.png
20191210-163441-553.png
20191210-164621-498.png
20191210-170248-775.png
20191210-170338-331.png
20191210-170438-437.png
20191210-170556-912.png

@PVince81 PVince81 force-pushed the separate-nav-from-app-switcher branch from bf13cb6 to 9f891fb Compare December 12, 2019 14:09
@PVince81
Copy link
Contributor Author

I've updated the design system now and aligned the styles a bit:

  • the new apps icon is there
  • also fixed the notifications logic so the bell appears again and the dropdown works

@ownclouders
Copy link
Contributor

💥 Acceptance tests Login failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7137/

20191212-141814-031.png
20191212-141909-813.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests RestrictSharing failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7137/

20191212-141750-549.png
20191212-141851-524.png
20191212-142104-199.png
20191212-142124-551.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Upload failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7137/

20191212-142240-262.png
20191212-142334-645.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests iPhone failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7137/

20191212-141727-025.png
20191212-141808-639.png
20191212-141849-275.png
20191212-141933-065.png
20191212-142013-259.png
20191212-142051-523.png
20191212-142127-156.png
20191212-142202-308.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingAutocompletion failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7140/

20191212-171231-517.png
20191212-171334-026.png
20191212-171416-873.png
20191212-171518-774.png
20191212-171924-045.png
20191212-172006-887.png
20191212-172049-668.png
20191212-172128-543.png
20191212-172507-936.png
20191212-172620-212.png
20191212-172815-182.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Resharing failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7140/

20191212-172040-576.png
20191212-172135-776.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingPermissionsUsers failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7140/

20191212-171311-708.png
20191212-171335-971.png
20191212-171415-764.png
20191212-171434-812.png
20191212-171453-400.png
20191212-171534-366.png
20191212-171552-880.png
20191212-171612-615.png
20191212-172157-332.png
20191212-172241-107.png
20191212-172324-049.png
20191212-172346-730.png
20191212-172405-640.png
20191212-172445-532.png
20191212-172736-881.png
20191212-172758-555.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests Trashbin failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7140/

20191212-171719-146.png
20191212-171809-267.png
20191212-171856-786.png
20191212-171945-285.png
20191212-172838-220.png
20191212-172838-470.png
20191212-172924-463.png
20191212-172924-702.png
20191212-173010-427.png
20191212-173010-767.png
20191212-173056-155.png
20191212-173056-378.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingPublic failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7140/

20191212-172036-894.png
20191212-172049-872.png

@PVince81 PVince81 added Status:Needs-Review Needs review from a maintainer and removed Status:In-Progress labels Dec 13, 2019
@PVince81 PVince81 mentioned this pull request Dec 13, 2019
Closed
@PVince81 PVince81 force-pushed the separate-nav-from-app-switcher branch from b7efbdc to 8ea2afc Compare December 16, 2019 20:18
@PVince81
Copy link
Contributor Author

rebased

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingNotifications failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7228/

20191216-203429-388.png
20191216-203510-874.png
20191216-203542-002.png
20191216-203613-901.png
20191216-203646-284.png
20191216-203717-457.png
20191216-203749-241.png
20191216-203820-892.png
20191216-203852-382.png
20191216-203924-310.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests SharingPublic failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7228/

20191216-204111-511.png
20191216-204121-480.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests iPhone failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7228/

20191216-204804-846.png
20191216-204825-041.png
20191216-205559-157.png
20191216-205639-340.png
20191216-205721-411.png
20191216-205800-919.png
20191216-205839-246.png
20191216-205916-593.png
20191216-205954-239.png
20191216-210033-241.png

@ownclouders
Copy link
Contributor

💥 Acceptance tests XGAPortrait failed. Please find the screenshots inside ...

https://drone.owncloud.com/owncloud/phoenix/7228/

20191216-205011-381.png
20191216-205031-638.png
20191216-205822-775.png
20191216-205859-425.png
20191216-205936-678.png
20191216-210015-301.png
20191216-210053-618.png
20191216-210133-504.png
20191216-210214-448.png
20191216-210313-789.png

@PVince81
Copy link
Contributor Author

New issues:

  • public link: tests/acceptance/features/webUISharingPublic/shareByPublicLink.feature:184
  • notifications:
    • tests/acceptance/features/webUISharingNotifications/shareWithGroups.feature:21
    • tests/acceptance/features/webUISharingNotifications/shareWithUsers.feature:15
    • tests/acceptance/features/webUISharingNotifications/shareWithUsers.feature:27
    • tests/acceptance/features/webUISharingNotifications/shareWithUsers.feature:36

@PVince81 PVince81 force-pushed the separate-nav-from-app-switcher branch from 8ea2afc to a5db0ea Compare December 17, 2019 11:24
@PVince81
Copy link
Contributor Author

Fixed public link header to not display menu button.
Fixed notifications to properly initialize after logging in.
Rebased + squashed.

DeepDiver1975
DeepDiver1975 requested changes Dec 17, 2019
View reviewed changes
config.json.sample Outdated Show resolved Hide resolved
config.json.sample Outdated
"applications": [{
"name": "Files",
"iconMaterial": "folder",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

icons need to be provided by the app - they have to be loaded by url

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will be resolved in adjusted sample config

src/components/ApplicationsMenu.vue Outdated Show resolved Hide resolved
src/components/UserMenu.vue Show resolved Hide resolved
src/store/config.js Show resolved Hide resolved
@PVince81 PVince81 force-pushed the separate-nav-from-app-switcher branch from a5db0ea to bfc6278 Compare December 17, 2019 13:45
@PVince81
Copy link
Contributor Author

PVince81 commented Dec 17, 2019 •
edited
Loading

Adjusted sample config.
Adjusted URL detection in icon field.
Answered comments.
Removed wrapping div.

@DeepDiver1975 @LukasHirt please mark comments as resolved in your re-review

@PVince81 PVince81 mentioned this pull request Dec 17, 2019
Closed
10 tasks
LukasHirt
LukasHirt approved these changes Dec 18, 2019
View reviewed changes
Copy link
Collaborator

@LukasHirt LukasHirt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good IMO 👍 Just some styling that could be improved but I guess that would be in some other PR.

@PVince81 PVince81 merged commit bc2e0f9 into master Dec 18, 2019
@delete-merged-branch delete-merged-branch bot deleted the separate-nav-from-app-switcher branch December 18, 2019 17:18
@PVince81 PVince81 mentioned this pull request Jan 7, 2020
Closed
24 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DeepDiver1975 DeepDiver1975 DeepDiver1975 requested changes

@LukasHirt LukasHirt LukasHirt approved these changes

Assignees

@PVince81 PVince81

Labels
Status:Needs-Review Needs review from a maintainer
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Separate app switcher from app specific navigation
4 participants
@PVince81 @ownclouders @DeepDiver1975 @LukasHirt