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

Add NcHeaderMenu #3489

Merged
merged 4 commits into from
Dec 6, 2022
Merged

Add NcHeaderMenu #3489

merged 4 commits into from
Dec 6, 2022

Conversation

skjnldsv
Copy link
Contributor

image
Since we're positioning it fixed on nextcloud the live example is not as great as this example, but it works out.

From https://github.com/nextcloud/server/blob/abaa3ef61b9a6177ca3fe965e837e6e9bca16a4b/core/src/views/UnifiedSearch.vue

@skjnldsv skjnldsv added the 3. to review Waiting for reviews label Nov 17, 2022
@skjnldsv skjnldsv added this to the 7.1.0 milestone Nov 17, 2022
@skjnldsv skjnldsv self-assigned this Nov 17, 2022
@skjnldsv skjnldsv requested review from PVince81 and Pytal and removed request for a team November 17, 2022 09:55
@skjnldsv skjnldsv added the accessibility Making sure we design for the widest range of people possible, including those who have disabilities label Nov 17, 2022
nickvergessen
nickvergessen previously requested changes Nov 17, 2022
View reviewed changes
Copy link
Contributor

@nickvergessen nickvergessen left a comment

Choose a reason for hiding this comment

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

I was gifted a similar component in the notifications app by @CarlSchwan
https://github.com/nextcloud/notifications/blob/master/src/Components/HeaderMenu.vue

Can we look that it can cover both cases?

@nickvergessen
Copy link
Contributor

Sorry... I read the sample not the actual template...

@skjnldsv
Copy link
Contributor Author

I was gifted a similar component in the notifications app by @CarlSchwan

Carl took mine from server 🙈
This is actually the reason I am putting this one here 😉

@Pytal Pytal mentioned this pull request Nov 17, 2022
Closed
3 tasks
@nickvergessen nickvergessen removed their request for review November 18, 2022 09:07
@Pytal Pytal mentioned this pull request Nov 26, 2022
Closed
@Pytal Pytal modified the milestones: 7.1.0, 7.2.0 Nov 30, 2022
@skjnldsv skjnldsv requested a review from artonge December 2, 2022 10:26
Pytal
Pytal approved these changes Dec 2, 2022
View reviewed changes
Copy link
Contributor

@Pytal Pytal left a comment

Choose a reason for hiding this comment

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

Apart from the minor eslint errors

@skjnldsv
Add NcHeaderMenu
4654ca8 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
Add focus trap
e47243a 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
Fix opened/closed event emit and slight refactor
e5875db 
Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@skjnldsv
Copy link
Contributor Author

skjnldsv commented Dec 2, 2022

Apart from the minor eslint errors

Fixed! 🙈

artonge
artonge approved these changes Dec 5, 2022
View reviewed changes
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Minor nitpick, but good otherwise :)

src/components/NcHeaderMenu/NcHeaderMenu.vue Show resolved Hide resolved
@skjnldsv
Do not double the focusTrap init $nextTick
b087224 
Signed-off-by: John Molakvoæ <skjnldsv@users.noreply.github.com>
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Dec 6, 2022
@skjnldsv skjnldsv merged commit 5aa06a9 into master Dec 6, 2022
@skjnldsv skjnldsv deleted the feat/headermenu branch December 6, 2022 07:39
@Pytal Pytal mentioned this pull request Dec 7, 2022
Closed
3 tasks
@skjnldsv skjnldsv added the enhancement New feature or request label Dec 9, 2022
@skjnldsv skjnldsv mentioned this pull request Dec 9, 2022
Merged
@raimund-schluessler raimund-schluessler mentioned this pull request Feb 13, 2023
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artonge artonge artonge approved these changes

@Pytal Pytal Pytal approved these changes

@nickvergessen nickvergessen nickvergessen left review comments

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

@PVince81 PVince81 Awaiting requested review from PVince81

Assignees

@skjnldsv skjnldsv

Labels
4. to release Ready to be released and/or waiting for tests to finish accessibility Making sure we design for the widest range of people possible, including those who have disabilities enhancement New feature or request
Projects
None yet
Milestone
7.2.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@skjnldsv @nickvergessen @artonge @Pytal