EnCiv / civil-pursuit

Other
8 stars 9 forks source link

Buttons #246

Open ooooorchid opened 1 week ago

ooooorchid commented 1 week ago
  1. Change padding to match visually to padding: 0.75rem 1.25rem Storyboard

  2. Increase space between underline to text-underline-offset: .25rem Make this change for all instances of underline Storyboard

  3. Add this instance into Storyboard (with plus sign)

ddfridley commented 1 week ago

@ooooorchid It's great to have feedback on all the design/implementation and great to have you doing it directly in github. I think for someone to work on them, it would really help to have the link to the storybook page/test you are referring to. And sometimes, a link to the figma page.

In the case of "Secondary Action" I'm confused.

https://654d6da9f019ad5dccb66b9b-yknwfelmpp.chromatic.com/?path=/story/button--secondary

Using the chrome dev console I see the padding around the button is 8px and 20px where are .5rem and 1.25rem. So I'm not sure what action to take here.

image

ddfridley commented 1 week ago

The On Done on Click test case https://654d6da9f019ad5dccb66b9b-yknwfelmpp.chromatic.com/?path=/story/button--on-done-clicked is using the base button component which doesn't have colors applied, to test the onDone function call by simulating a user clicking on the button. It's not referring to a particular state. So I don't think there's a problem here.

ddfridley commented 1 week ago

On the underline, https://654d6da9f019ad5dccb66b9b-yknwfelmpp.chromatic.com/?path=/story/button--hover-test

how about .2rem image

Or .25rem image