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

fix(NcActions): clear focus trap and move focus only if elements are existing in the DOM #5345

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

Antreesy
Copy link
Contributor

@Antreesy Antreesy commented Mar 5, 2024

☑️ Resolves

There are couple of places, where triggering an action in NcActions (by click, for example) leads to unmounting / breaking re-rendering of NcActions immediately. (Examples in Talk: stop the screensharing (stable28), end the call (main)). In such cases, after nextTick ref elements (popover, menuButton) are no longer present in the DOM (undefined), thus attempt to call methods on them lead to an error (see issues).

Popover has clearFocusTrap in beforeDestroy hook, so changes should be safe to apply (regarding previous PR)

🖼️ Screenshots

No behaviour change, just clean console 🧹

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 3️⃣ Backport to next requested with a Vue 3 upgrade

…existing in the DOM

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy added 3. to review Waiting for reviews feature: actions Related to the actions components regression Regression of a previous working feature labels Mar 5, 2024
@Antreesy Antreesy added this to the 8.8.2 milestone Mar 5, 2024
@Antreesy Antreesy requested review from susnux and ShGKme March 5, 2024 10:00
@Antreesy Antreesy self-assigned this Mar 5, 2024
@Antreesy
Copy link
Contributor Author

Antreesy commented Mar 5, 2024

/backport to next

@Antreesy Antreesy merged commit 2cafc03 into master Mar 5, 2024
18 checks passed
@Antreesy Antreesy deleted the fix/5292/focus-trap-if-existing branch March 5, 2024 18:01
@juliusknorr juliusknorr mentioned this pull request Mar 6, 2024
@juliusknorr juliusknorr modified the milestones: 8.8.2, 8.9.0 Mar 6, 2024
@ShGKme
Copy link
Contributor

ShGKme commented Mar 6, 2024

Do not forget add bug for fixes or enh for new features labels for changelog generation 😛

@Antreesy
Copy link
Contributor Author

Antreesy commented Mar 6, 2024

for changelog generation

I thought regression is respected as well, will do in future

@susnux susnux added the bug Something isn't working label Mar 6, 2024
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: actions Related to the actions components regression Regression of a previous working feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[NcActions] popover is removed before clearForcusTrap
5 participants