-
Notifications
You must be signed in to change notification settings - Fork 93
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: Add NcChip component #5686
Conversation
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.
Everything looks great
…fault clickable area Signed-off-by: Ferdinand Thiessen <[email protected]>
(Fixed stylelint issue) |
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.
Very nice <3
I would just up the trailing padding when there's text only. 6px instead of 4 should work. Possibly we'll have to reduce the icon size a bit too but let's see it in action first :)
Signed-off-by: Ferdinand Thiessen <[email protected]>
…nextcloud/router` in styleguide Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Ferdinand Thiessen <[email protected]>
Signed-off-by: Ferdinand Thiessen <[email protected]>
/backport to next |
Just so you know the interraction in that component doesn't pass accessibility requirements. |
For the reference: https://www.w3.org/WAI/WCAG21/Understanding/target-size.html
So, the question is how it is supposed to be used. As I understand, it is kind of an alternative to |
No, it does satisfy the WCAG rules, as we only focus on AA not AAA, so the minimum target size is 24px x 24px. So the current design roadmap is to shrink our defaul clickable area (probably to 34px x 34px) and only use big buttons (there will be large variant of buttons) that use 44px x 44px area.
No it is ment for e.g. the unified search to display the current search filter. |
I'm more dubious about the menu within that new bubble. That would be very hidden and I doubt this will pass WCAG :) The inline close icon is probably already discutable 🤔 |
There is only a trigger button that has a size of 24px which is the minimum allowed for accessibility, so this will pass WCAG. |
It will pass, but I agree that it's definitely not a happy place for action menus. I would limit the usage to a close button only @susnux Do you have any other use cases in mind for advanced usage? |
@susnux is the ncBubble deprecated then? |
Currently it is a general purpose component, as the I had not planned to deprecate the bubble, but would be possible if this is desired. PS: If you like to use the chip inline you need to add |
☑️ Resolves
A "modern" alternative to
NcUserBubble
.The intended use case is for searches or filters to show as the currently active filters.
The current default size is 24px to align with UI changes (cc @marcoambrosini ).
It allows to set an icon (no legacy classes but icon slot or prop), the text and optionally actions (by default only the close button).
🖼️ Screenshots
🏁 Checklist
next
requested with a Vue 3 upgrade