-
Notifications
You must be signed in to change notification settings - Fork 24
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
Tag - confirm which icon to use for close #3256
Comments
It is recommended to use the 'Close - Core Icon' for the tags than 'Close - Form Icon'.
Based on this I will be creating an issue for 'Tag - Update Figma component to use "Close" icon.' |
@KennyAtHPE If you can give a feedback on this. |
Figma related ticket (Tag -Update Figma component to use "Close" icon) #3467 Grommet related ticket (Tag - Update v5 theme to use "Close" icon.) grommet/grommet#6870 |
@ashifalinadaf Can you include a link please of where I can find the tags. |
Nvm, found it in the linked ticket above. Nice work Asif, all of the light and dark mode close buttons are using the correct close/core icon. One thing that came to mind when I was looking at these is whether we want to use the actual default button for the close button part. The only thing is that our smallest icon only button (small) is 36x36px where as the tag component in sizes xs-m are using 18x18px for the close button. Might be worth a discussion? |
I agree @vavalos5, |
The Tag remove icon sizing is currently coming from grommet's base theme. I think for the scope of this ticket, let's align with what is currently defined and represented in the theme. If we want to further discuss the sizing/changes, a follow-on ticket can be filed. Doesn't feel high priority at the moment though. The sizing should be:
|
@ashifalinadaf the icon sizes are correct. I was referring to the idea of having a potential conversation with a dev or the team in regards to adding smaller sizes to our current button components so we can use the correct ones and be aligned to the theme. @taysea I agree it's not a priority at the moment, but something we might want to consider in the future. |
In that case all the 'form/close' icons were replaced by 'core/close' icons. We can include all the comments related to 'using default buttons for tags' in a new ticket. |
Closing this ticket. The initial deliverables to "Create issues" have been completed and documented in the issue description. |
v5 generally moves away from Form[IconName]. Tag still uses FormClose. Leave as is? Or use Close?
ToDos:
The text was updated successfully, but these errors were encountered: