FatimaGC / Portfolio

A portfolio of my work
1 stars 0 forks source link

Feature/secondary button variant #14

Closed FatimaGC closed 1 year ago

FatimaGC commented 1 year ago

@kee-oth Do you think I'm good to merge this?

kee-oth commented 1 year ago

@FatimaGC

I'm seeing some differences between the Primary and Secondary buttons:

  1. Primary is rounded while secondary is squared off
  2. Secondary "border" (it's actually an outline) makes the overall visual dimensions of the button larger. I recommend using the border property for this instead of outline. You can't make an outline rounded and border actually counts for the size of the button, which we'd want for this (otherwise, other elements will visually overlap with the button since the outline doesn't take up "physical" space, other elements act like it's not even there)
  3. The hover state of the secondary button looks odd because of the outline not being rounded.

Primary

Screenshot 2023-05-02 at 7 31 04 PM

Secondary

Screenshot 2023-05-02 at 7 31 15 PM

Secondary hover

Screenshot 2023-05-02 at 7 36 57 PM
kee-oth commented 1 year ago

@FatimaGC There's also no shadow on secondary variant. Let's make sure that it's there for all the variants 👍

FatimaGC commented 1 year ago

@kee-oth I checked and this is how things are looking on my end:

Secondary

Screenshot 2023-05-03 at 7 39 50 PM

Secondary Hover

Screenshot 2023-05-03 at 7 39 55 PM

Not sure why there's a discrepancy

kee-oth commented 1 year ago

@FatimaGC

Looking better! But the buttons are different sizes, let's figure out what's going on there and fix it. Notice how the dimensions of the grid/box the same but in the Secondary button the border is on the outside of the lines while on the Primary button the border is on the inside:

Primary

Screenshot 2023-05-04 at 9 43 29 PM

Secondary

Screenshot 2023-05-04 at 9 43 52 PM
FatimaGC commented 1 year ago

@FatimaGC

Looking better! But the buttons are different sizes, let's figure out what's going on there and fix it. Notice how the dimensions of the grid/box the same but in the Secondary button the border is on the outside of the lines while on the Primary button the border is on the inside:

Primary Screenshot 2023-05-04 at 9 43 29 PM

Secondary Screenshot 2023-05-04 at 9 43 52 PM

@kee-oth Is the solution to this to add box-sizing: border-box to .button class?

kee-oth commented 1 year ago

@FatimaGC Looks good to merge after that one change 💪