Conversation
4c21322 to
c2c704a
Compare
|
/backport to stable25 |
nickvergessen
left a comment
There was a problem hiding this comment.
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
apps/user_status/src/menu.js
Outdated
| displayName: avatarDiv.dataset.displayname, | ||
| disableMenu: true, | ||
| disableTooltip: true, | ||
| const mountMenu = () => { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
Just some smaller comments, but looks good otherwise 👍
a2726b4 to
77771f5
Compare
1e6130b to
694f351
Compare
694f351 to
79bd9aa
Compare
|
Rebased |
szaimen
left a comment
There was a problem hiding this comment.
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).
79bd9aa to
71e9415
Compare
71e9415 to
5102ecc
Compare
Signed-off-by: Christopher Ng <chrng8@gmail.com>
5102ecc to
c779982
Compare
Summary
Move PHP user menu markup to Vue wrapped by the focus trapped NcHeaderMenu component
Requires
Allow empty route for navigation entries #36449Related
Checklist