Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Design System: Refactor IconButton and update documentation #66774
Design System: Refactor IconButton and update documentation #66774
Changes from 22 commits
c151b6b
4edaf78
c584610
4acfb78
19aba85
2378f13
890f9b3
f5bb0cc
d640c00
6bcae65
17e23e2
e618957
89278eb
40550c2
55ca13a
a08eff5
27c1ede
d6f124c
1923605
f4275d3
cf28f1f
2f7ce5a
8e8f175
20a5c35
9ea630a
f77eabc
024e57c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
we should make it use the theme here, so 2 * theme.spacing(0.5)
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.
Unfortunately
theme.spacing()
is returning a string including 'px'.I could do something like
parseInt(theme.spacing(0.5).substring(0))
but this is cumbersome. What do you think?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.
Oh thats true... We could also use
theme.spacing.gridSize
which returns8
? And keep this in mind when we do spacing tokensThere 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.
I changed it to
theme.spacing.gridSize
, modified the related comment in the code and added a reminder to the related epic for spacing tokens.