-
Notifications
You must be signed in to change notification settings - Fork 12
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
feat(tab): adds closable attribute and close event (VIV-2030) #1915
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1915 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 123 349 +226
Lines 1562 6773 +5211
Branches 108 869 +761
===========================================
+ Hits 1562 6773 +5211
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't went over all the code yet, tested keyboad navigation and screen-reader and its not 100%.
When tab is removed (also - there's no way to know it is removable), the focus disappears, shouldn't it go the the latest tab?
Also - if all tabs are deleted it creates a strange use case:
maybe we need to add that if only one is left it can't be removed?
Co-authored-by: Rachel Bratt Tannenbaum <rachelbt@users.noreply.github.com>
Co-authored-by: Rachel Bratt Tannenbaum <rachelbt@users.noreply.github.com>
id="close-btn" | ||
@click="${(x, c) => x._handleCloseClick(c.event)}" | ||
> | ||
<${iconTag} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can't we use here the affix pattern as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 just ine comment about the icon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great
No description provided.