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: cv-tabs fails in focusout event when using router-link in a tab #1084

Merged
merged 4 commits into from Dec 11, 2020
Merged

Conversation

Fontinalis
Copy link
Contributor

Closes #1083

This PR changes the cv-tabs component, adds the event listeners on mount and removes them in beforeDestroy, doing this with this.$refs.tabs where tabs is the root div which previously had the listeners attached as @focusout="onFocusout" @focusin="onFocusin".

Changelog

M packages/core/src/components/cv-tabs/cv-tabs.vue

@netlify
Copy link

netlify bot commented Nov 26, 2020

✔️ Deploy preview for carbon-components-vue ready!

🔨 Explore the source changes: abf7996

🔍 Inspect the deploy logs: https://app.netlify.com/sites/carbon-components-vue/deploys/5fd396665538770007db5b25

😎 Browse the preview: https://deploy-preview-1084--carbon-components-vue.netlify.app

@Fontinalis
Copy link
Contributor Author

Fontinalis commented Nov 26, 2020

Hey @lee-chase ,

First, thank you so much for maintaining this package, it's awesome and we love it. On the other hand, when you have a little time, can you please check this PR? Based on my tests, it fixed the issue and the code is right, but of course, I'm happy to take some comments.

@Fontinalis Fontinalis changed the title fix #1083: cv-tabs fails in focusout event when using router-link in a tab fix: cv-tabs fails in focusout event when using router-link in a tab Nov 30, 2020
@Fontinalis
Copy link
Contributor Author

Hey @lee-chase,
Can you please review this? I know you are probably busy, and I do respect your time. We're just in a situation where this issue is keep popping up and slows us down.. In functionality, it doesn't change anything, the listeners are working the same way, it just removes the listeners from the cv-tabs component before destroying it so this error is not thrown.
Thanks again for your work on the project!

Copy link
Member

@lee-chase lee-chase left a comment

Choose a reason for hiding this comment

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

Looks good although I'm usually keen to observer an issue before trying to fix it. Can you create a code sandbox demonstrating the issue?

Are you using the tabs as you remain navigation? Or do the tabs change a navigation parameter?

Just trying to understand you use case.

@Fontinalis
Copy link
Contributor Author

Fontinalis commented Dec 11, 2020

Here's what I meant https://codesandbox.io/s/eager-poitras-7lo88?file=/pages/index.vue

Edit:
This is the issue I found and if you click on the "about" link in tab1, it will show an error from the cv-tabs component that prevents the navigation to the /about page

@lee-chase lee-chase merged commit 91adefb into carbon-design-system:master Dec 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cv-tabs fails in focusout event when using router-link in a tab
2 participants