docs(button): expose tertiary button and restrict icon button variants #5201
Conversation
ghost
requested review from
|
Deploy preview for the-carbon-components ready! Built with commit 95f7ba3 https://deploy-preview-5201--the-carbon-components.netlify.com |
|
Deploy preview for carbon-elements ready! Built with commit 95f7ba3 |
|
Deploy preview for carbon-components-react failed. Built with commit 95f7ba3 https://app.netlify.com/sites/carbon-components-react/deploys/5e34c596995e7600070a0072 |
There was a problem hiding this comment.
Vanilla:
- The active state is blocking out the text in the button. the icon only button is correct though.
React:
-
The active state does not change the background of the container and it is making the text turn black. It should only be changing the background color to the
active-tertiarycolor token. The icon only button is correct though.
-
Also I noticed when switching the default buttons from primary to secondary to tertiary, the tertiary buttons jump a bit in size horizontally. the tertiary buttons should remain the same size as they are in primary and secondary.
emyarod
force-pushed
the
5199-tertiary-button-docs
branch
from
6051304 to
160d79b
Compare
|
vanilla should be correct now the react issues are mainly because the base elements of the examples you tested are not actually buttons (we're showing some examples of other elements being styled like buttons). mainly we want to check if the element labeled "Button" is correct, since using a different base element can change how active and focus states are applied the change in width is due to the border width changing from 3px in the other variants to 1px in the tertiary and ghost variants. does that mean the padding needs to change for the 1px border button variants? |
There was a problem hiding this comment.
Okay thanks @emyarod that makes sense ! All of the changes you made look good.
-
The tertiary icon button is getting a weird double focus outline in react and vanilla.
-
In dark themes, the active state is getting a white border around the active background color, there shouldn't be a visible border.
|
tertiary button width should be the same as the other button variants now fixed the double focus outline ref #4867 for the dark theme active state border, what should the token be there? it looks like it's just inheriting |
|
the token is right for the border, its just that that border should not be showing up on active state, it should be a solid background. Could the active background overlap that border somehow? Its not happening in the light themes. |
|
@emyarod Is this PR ready for re-review? Thanks! |
|
@asudoh yes it is ready for review |
|
Got a green light from @emyarod to move this to "ready to merge" state. |
Closes #5199
This PR exposes the tertiary buttons in the React storybook and removes incorrect icon button variants from the React and vanilla docs
Testing / Reviewing
Ensure that only the correct button knobs and variants are exposed in the docs