-
-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Support badge size for action components #12784
Conversation
I've also added this prop to the default button component as well. Instead of hardcoding the badge as |
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.
Please do the same for the link component :)
Could you share the use case for this, @tnylea? |
Thanks for the feedback guys. I'll be adding this to the link component as well. I see that I need to make some layout modifications based on the size. When we increase the badge size to an XL, it looks like it sits too low on the element. So, I'll work to get this to show correctly for all sizes. Just wanted to keep you posted that this is on my radar and I'll work on getting this updated on the weekend or upcoming week. I'll be sure to include screenshots of all elements and all sizes of the badge. Hope you guys are having a great Friday! Talk soon! |
Yeah that's exactly what I meant. |
Hey guys, This PR should be good to go unless there is anything else you see on your end. I have made sure to merge the latest Here is the screenshot of the original badge size from this PR: This is for the Let me know if these look good or if we need any other modifications :) Appreciate it! |
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.
Thanks for the update and screenshots, @tnylea!
Has the horizontal displacement of the badge changed from the original position? I think it's a little too far away from the link specifically. Not sure though, so that's why I'm asking. Also I think it should be tailored for every badge size.
PS: Could you update the screenshots for the link, since they all use the sm
badge size.
Thanks @zepfietje, I'll be circling back around to this today 😉 |
Important I'm not sure why PHPStan tests are failing. They seem to be throwing an error on some of the files that I did not even change 🤷♂️ Any help on this would be appreciated 😊 Right you were @zepfietje, I needed to add some additional styles to the link badge in order to get it to look correct with all different sizes. I see that the options for badge size are Here is how it looks right now. The last one may look a little weird because I didn't add enough padding to the middle and bottom element, but you can see that the badges look good with all sizes. I doubt that many people will use the Let me know if there's anything else you would like me to update and I can make those changes. Looking forward to getting this merged in 😉 Thanks! Talk to you soon. |
@zepfietje Tests seem to be passing now. Let me know if there's anything additional you want me to add to this one and I can make it happen. Thanks! Look forward to hearing from you soon. |
Perfect, thanks Tony! Will be reviewing and merging this next. 👍 |
Great! Thanks @zepfietje 😊 |
Small change, big impact. Thank you. I have been needing proper spacing for my badges numerous times. I suppose now the "hacking" stops 👍💪 |
Hey @zepfietje, let me know if there's anything I can do to help get this merged 😉 Thanks! |
I'm going through issues and PRs now, so will merge this in a bit! 👍 |
@tnylea, next time please keep the PR template and follow the required steps (like fixing code style by running |
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.
Thanks for the groundwork on this one, @tnylea!
I've made it more responsive by using only relative positioning.
Ready to be merged now. 👌
In this PR I've added the
badgeSize
prop to the Icon Button component. This will allow the user the ability to modify the size of the badge by passing it through as a prop to the parent Icon Button component 👍Let me know if you have any questions about this PR. Appreciate it.