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

Update Components: Button, Chip and Tooltip #204

Merged
merged 13 commits into from
Mar 20, 2020
Merged

Conversation

daigof
Copy link
Contributor

@daigof daigof commented Mar 13, 2020

  • Buttons: Update all buttons to have border radius: 4px.
  • Chip: new styles, added default and secondary status. deprecated pending and closed status, but they will fallback to default styles if someone provides an incorrect status
  • Tooltip: update styles. Fixed arrow center positioning (now the arrow wont get in way of letters and is perfect centered). New isSmall prop for smaller tooltips.

Other minor fixes

  • Callout: fixed doc to use 48x48 glyph
  • Indicator: refactored to use same technique as chip using display: inline-text instead of width:content-fit
  • Edit Storybook build (webpack config) that makes images broken. Compare Avatar component in PROD the images are all broken, this fixes that.

Preview link

https://curology-radiance-pr-204.herokuapp.com/

@snags88 snags88 temporarily deployed to curology-radiance-pr-204 March 13, 2020 18:24 Inactive
Copy link
Contributor

@benkolde benkolde left a comment

Choose a reason for hiding this comment

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

Minor thing, but for isSmall can we add vertical padding of 2px

For both, can we make the trigger have borderRadius: 1 and change the trigger width to 8px

@daigof
Copy link
Contributor Author

daigof commented Mar 13, 2020

Minor thing, but for isSmall can we add vertical padding of 2px

For both, can we make the trigger have borderRadius: 1 and change the trigger width to 8px

@benkolde What is the trigger? did you mean the arrow? I made the arrow 8px dimension now but I dont know how border radius would affect it

@snags88 snags88 temporarily deployed to curology-radiance-pr-204 March 13, 2020 20:04 Inactive
@benkolde
Copy link
Contributor

Minor thing, but for isSmall can we add vertical padding of 2px
For both, can we make the trigger have borderRadius: 1 and change the trigger width to 8px

@benkolde What is the trigger? did you mean the arrow? I made the arrow 8px dimension now but I dont know how border radius would affect it

I just want the arrow to look like this

image

@daigof
Copy link
Contributor Author

daigof commented Mar 19, 2020

@benkolde @ida-strom this is ready for re-review. One thing I noticed is that the tooltip extends to the width of the Trigger element, if wrongly set or unchecked it can go very wide. I suggest putting a max-width: 327px

Unchecked can grow as:
Screen Shot 2020-03-19 at 10 38 12

With Max-width Fix:
Screen Shot 2020-03-19 at 10 37 54

@daigof daigof requested a review from benkolde March 19, 2020 13:41
@benkolde
Copy link
Contributor

@benkolde @ida-strom this is ready for re-review. One thing I noticed is that the tooltip extends to the width of the Trigger element, if wrongly set or unchecked it can go very wide. I suggest putting a max-width: 327px

Unchecked can grow as:
Screen Shot 2020-03-19 at 10 38 12

With Max-width Fix:
Screen Shot 2020-03-19 at 10 37 54

Can this be a prop? @daigof I think having the option for it to expand would be great

@benkolde
Copy link
Contributor

Can you build a new test environment for this?

@snags88 snags88 temporarily deployed to curology-radiance-pr-204 March 19, 2020 16:45 Inactive
@daigof
Copy link
Contributor Author

daigof commented Mar 19, 2020

@benkolde I added a new optional prop for restricted width (327px) with default false value so it doesn't impact existing implementations.

Deployed a review app here https://curology-radiance-pr-204.herokuapp.com/

@daigof
Copy link
Contributor Author

daigof commented Mar 20, 2020

@benkolde Updated the border radius of small tooltip to 4px.

https://curology-radiance-pr-204.herokuapp.com/?path=/story/tooltip--usage

@daigof daigof requested a review from benkolde March 20, 2020 12:58
@daigof daigof merged commit 2c1b7bb into master Mar 20, 2020
@daigof daigof deleted the update-tooltip-chip-buttons branch March 20, 2020 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants