Skip to content

Port user menu to Vue - #36232

Merged
Pytal merged 1 commit into
masterfrom
enh/a11y-user-menu
Feb 11, 2023
Merged

Port user menu to Vue#36232
Pytal merged 1 commit into
masterfrom
enh/a11y-user-menu

Conversation

@Pytal

@PytalPytal commented Jan 19, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Summary

Move PHP user menu markup to Vue wrapped by the focus trapped NcHeaderMenu component

BeforeAfter
imageimage

Requires

Related

Checklist

@PytalPytal added the 2. developing Work in progress label Jan 19, 2023
@PytalPytal self-assigned this Jan 19, 2023
@PytalPytal mentioned this pull request Jan 19, 2023
Closed
2 tasks
@Pytal
Pytalforce-pushed the enh/a11y-user-menu branch 5 times, most recently from 4c21322 to c2c704aCompareJanuary 25, 2023 02:12
@PytalPytal changed the title [WIP] Port user menuPort user menu to VueJan 25, 2023
@Pytal

Pytal commented Jan 25, 2023

Copy link
Copy Markdown
MemberAuthor

/backport to stable25

@Pytal
Pytal marked this pull request as ready for review January 25, 2023 02:20
@PytalPytal added enhancement 3. to review Waiting for reviews accessibility and removed 2. developing Work in progress labels Jan 25, 2023
@PytalPytal added this to the Nextcloud 26 milestone Jan 25, 2023
nickvergessen
nickvergessen reviewed Jan 25, 2023
View reviewed changes
Comment threadcore/src/views/UserMenu.vue Outdated
nickvergessen
nickvergessen reviewed Jan 25, 2023
View reviewed changes

@nickvergessennickvergessen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not breaking the external sites app, if that is why my review was requested.
From the technical changes in this PR I'm the wrong person to judge

juliusknorr
juliusknorr reviewed Jan 25, 2023
View reviewed changes
Comment threadlib/private/TemplateLayout.php
juliusknorr
juliusknorr reviewed Jan 25, 2023
View reviewed changes
Comment threadcore/templates/layout.user.php Outdated
juliusknorr
juliusknorr reviewed Jan 25, 2023
View reviewed changes
Comment threadapps/user_status/src/menu.js Outdated
displayName: avatarDiv.dataset.displayname,
disableMenu: true,
disableTooltip: true,
const mountMenu = () => {

@juliusknorrjuliusknorrJan 25, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we be fully sure that the UserMenu has already been rendered at this point? Otherwise this might need some way of handling to not end up with a menu without the profile link which might become hard to debug later on.

@PytalPytalJan 26, 2023

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Should be as this is only executed when UserMenu has been mounted and emits core:user-menu:mounted

This only works as long as apps/user_status/src/menu.js is loaded before core/src/components/UserMenu.js, but if UserMenu.js loads and emits core:user-menu:mounted before menu.js can subscribe to the event then it wouldn't

For this I adjusted the logic to handle both cases

juliusknorr
juliusknorr reviewed Jan 25, 2023
View reviewed changes

@juliusknorrjuliusknorr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just some smaller comments, but looks good otherwise 👍

@Pytal
Pytalforce-pushed the enh/a11y-user-menu branch 2 times, most recently from a2726b4 to 77771f5CompareJanuary 26, 2023 02:17
skjnldsv
skjnldsv requested changes Jan 26, 2023
View reviewed changes
Comment threadapps/user_status/src/menu.js Outdated
Comment threadcore/src/views/UserMenu.vue Outdated
@Pytal
Pytal changed the base branch from enh/allow-empty-route-xml to enh/allow-no-routeFebruary 3, 2023 01:25
@Pytal
Pytalforce-pushed the enh/a11y-user-menu branch from 1e6130b to 694f351CompareFebruary 3, 2023 01:26
@skjnldsvskjnldsv added 2. developing Work in progress 3. to review Waiting for reviews and removed 3. to review Waiting for reviews 2. developing Work in progress labels Feb 3, 2023
Base automatically changed from enh/allow-no-route to masterFebruary 3, 2023 11:54
@Pytal
Pytalforce-pushed the enh/a11y-user-menu branch from 694f351 to 79bd9aaCompareFebruary 4, 2023 00:23
@Pytal

Pytal commented Feb 4, 2023

Copy link
Copy Markdown
MemberAuthor

Rebased

@nickvergessen
nickvergessen removed their request for review February 6, 2023 09:50
@miaulalala
miaulalala removed their request for review February 7, 2023 13:00
szaimen
szaimen approved these changes Feb 7, 2023
View reviewed changes

@szaimenszaimen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested together with nextcloud/firstrunwizard#816 and works now but didnt review the code.

(Looks like the internal server issue was somehow caused by the viewer app not being installed).

@Pytal
Pytalforce-pushed the enh/a11y-user-menu branch from 79bd9aa to 71e9415CompareFebruary 9, 2023 18:59
@PytalPytal added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 9, 2023
@Pytal
Port user menu to Vue
c779982
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal
Pytal merged commit b59b8fd into masterFeb 11, 2023
@Pytal
Pytal deleted the enh/a11y-user-menu branch February 11, 2023 03:22
@backportbot-nextcloudbackportbot-nextcloudBot mentioned this pull request Feb 11, 2023
Merged
1 task
@blizzzblizzz mentioned this pull request Feb 13, 2023
Merged
@joshtrichardsjoshtrichards mentioned this pull request Feb 1, 2024
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessennickvergessennickvergessen left review comments
@artongeartongeartonge approved these changes
@szaimenszaimenszaimen approved these changes
@JuliaKirschenheuterJuliaKirschenheuterAwaiting requested review from JuliaKirschenheuter
@ChristophWurstChristophWurstAwaiting requested review from ChristophWurst
@tcitworldtcitworldAwaiting requested review from tcitworld
@skjnldsvskjnldsvAwaiting requested review from skjnldsv
@nimishavijaynimishavijayAwaiting requested review from nimishavijay
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

@PytalPytal

Labels

4. to releaseReady to be released and/or waiting for tests to finishaccessibilityenhancement

Projects

None yet

Milestone

Nextcloud 26

Development

Successfully merging this pull request may close these issues.

7 participants

@Pytal@szaimen@nickvergessen@juliusknorr@artonge@skjnldsv@nimishavijay