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.

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

[next] fix(NcModal): temporary deactivate focus-traps on modal open #5852

Merged
merged 3 commits into from
Jul 23, 2024

Conversation

backportbot[bot]
Copy link

@backportbot backportbot bot commented Jul 22, 2024

Backport of PR #5783

@backportbot backportbot bot requested review from ShGKme, susnux and Antreesy July 22, 2024 15:34
@backportbot backportbot bot added bug Something isn't working 3. to review Waiting for reviews feature: app-navigation Related to the app-navigation component feature: modal Related to the modal component labels Jul 22, 2024
Comment on lines 12 to 30
<div class="navigation__header">
<NcTextField :value.sync="searchValue" label="Search …" />
<NcActions>
<NcActionButton close-after-click @click="showModal = true">
<template #icon>
<IconCog />
</template>
App settings (close after click)
</NcActionButton>
<NcActionButton @click="showModal = true">
<template #icon>
<IconCog />
</template>
App settings (handle only click)
</NcActionButton>
</NcActions>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

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

@Antreesy when you handle the .sync could you also rephrase this part to not have something like search? Because search should only be handled by NcAppNavigationSearch and we should not have examples doing it other

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't you mind, if I forward-port it as-is, then replace this example on master and then push to the next afterwards?

@susnux susnux merged commit 5704773 into next Jul 23, 2024
20 checks passed
@susnux susnux deleted the backport/5783/next branch July 23, 2024 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working feature: app-navigation Related to the app-navigation component feature: modal Related to the modal component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants