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 trap stack util to public API #3516

Closed
wants to merge 1 commit into from
Closed

Add trap stack util to public API #3516

wants to merge 1 commit into from

Conversation

Pytal
Copy link
Contributor

@Pytal Pytal commented Nov 24, 2022

For use in server and apps

@Pytal
Add trap stack util to public API
ee71b24 
Signed-off-by: Christopher Ng <chrng8@gmail.com>
@Pytal Pytal added enhancement New feature or request 3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Nov 24, 2022
@Pytal Pytal added this to the 7.2.0 milestone Nov 24, 2022
@Pytal Pytal self-assigned this Nov 24, 2022
skjnldsv
skjnldsv requested changes Nov 24, 2022
View reviewed changes
Copy link
Contributor

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

Can you point or explain a context where we would use that in server or an app?
Assuming all the elements requiring a focus trap are vue components, doing this should not be necessary

@Pytal
Copy link
Contributor Author

Pytal commented Nov 24, 2022

Can you point or explain a context where we would use that in server or an app? Assuming all the elements requiring a focus trap are vue components, doing this should not be necessary

Could be used in nextcloud/server#35381 as the user and contacts menus are not in Vue

@skjnldsv
Copy link
Contributor

I would migrate them to vue and use the HeaderMenu then, no?

@Pytal
Copy link
Contributor Author

Pytal commented Nov 26, 2022

I would migrate them to vue and use the HeaderMenu then, no?

The menus have been using the legacy js implementation for awhile so if the component is flexible enough then sounds good!

Would then need #3489 first

@skjnldsv
Copy link
Contributor

There was an attempt:

@Pytal
Copy link
Contributor Author

Pytal commented Dec 9, 2022

Not needed as menus will use NcHeaderMenu

@Pytal Pytal closed this Dec 9, 2022
@skjnldsv skjnldsv deleted the enh/export-focus-trap-util branch December 9, 2022 06:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@skjnldsv skjnldsv skjnldsv requested changes

@GretaD GretaD GretaD approved these changes

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

Assignees

@Pytal Pytal

Labels
3. to review Waiting for reviews 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.
3 participants
@Pytal @skjnldsv @GretaD