Skip to content
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

fix(code-snippet): update component to spec #7214

Merged
merged 15 commits into from
Nov 12, 2020

Conversation

emyarod
Copy link
Member

@emyarod emyarod commented Nov 3, 2020

Part of #7159

This PR adds an active style to the code snippet copy buttons and repositions the button to be aligned with the "show more" chevron in multiline snippets.

Changelog

New

  • copy button active styles

Changed

  • multiline code snippet copy button positioning
  • light hover token for copy button

Testing / Reviewing

Confirm the code snippet matches the spec

@emyarod emyarod changed the title 7159 code snippet fixes fix(code-snippet): update component to spec Nov 3, 2020
@netlify
Copy link

netlify bot commented Nov 3, 2020

Deploy preview for carbon-elements ready!

Built with commit d779453

https://deploy-preview-7214--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented Nov 3, 2020

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit d779453

https://deploy-preview-7214--carbon-components-react.netlify.app

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • The light prop needs to also have an active state change for the copying functionality for inline/single line/multi-line code snippets. It is not showing up.

  • I think the multi-line code snippet copy button is too large which is why it is misaligning with the ghost button below it. It needs to be a 32px icon button instead of 40px.
    Artboard

@laurenmrice
Copy link
Member

laurenmrice commented Nov 10, 2020

So it was noticed that we need to make a token for active-light-ui for the light prop to achieve the correct colors.
$active-light-ui

Also for the current hover-light-ui token, it would be better to have calculated hover values for the dark themes like we do for the light themes.
$hover-light-ui

@emyarod
Copy link
Member Author

emyarod commented Nov 11, 2020

new active token and hover token updates will be here #7275

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the light prop and hide copy functionalities are both on, the inline code snippet is getting a hover. There should be no hover state when hide copy is on.
Nov-12-2020 11-31-32

Everything else looks great!

Copy link
Member

@laurenmrice laurenmrice left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Thank you 🙌🏻

@kodiakhq kodiakhq bot merged commit caea2ab into carbon-design-system:master Nov 12, 2020
@emyarod emyarod deleted the 7159-code-snippet-fixes branch November 12, 2020 20:17
@emyarod emyarod mentioned this pull request Nov 12, 2020
59 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants