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(ffe-buttons): endre line-height i basebutton #2139

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

HeleneKassandra
Copy link
Contributor

Beskrivelse

Endrer line-height i base button til å være 1.5 ch.

Motivasjon og kontekst

Px verdien fungerer litt dårlig på høy tekstzoom, så tror 1.5ch vil skalere bedre.
Dette ser også til å fikse ett issue der deler av bokstavene går "inn" i border på knapper ved høy zoom.

Før:
Skjermbilde 2024-07-02 kl  13 21 49

Etter:
Skjermbilde 2024-07-02 kl  13 22 20

Testing

@HeleneKassandra HeleneKassandra requested a review from a team as a code owner July 2, 2024 11:25
Copy link

github-actions bot commented Jul 2, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-2139.westeurope.2.azurestaticapps.net

@tuva-odegard
Copy link
Contributor

tuva-odegard commented Jul 2, 2024

Kan være noe Mac-greier, men det ser ikke ut som at det ble helt riktig hos meg.. Endringen førte til at knappen ble 5px lavere, og jeg får ikke samme resultatet som deg når jeg zoomer (øke tekststørrelsen)
image
Her er disse endringene til venstre og live til høyre, med 150% zoom (chrome)

@HeleneKassandra HeleneKassandra force-pushed the change-base-button-line-height-to-ch branch from 6f0d20c to cbb723f Compare July 2, 2024 11:47
@HeleneKassandra
Copy link
Contributor Author

Kan være noe Mac-greier, men det ser ikke ut som at det ble helt riktig hos meg.. Endringen førte til at knappen ble 5px lavere, og jeg får ikke samme resultatet som deg når jeg zoomer (øke tekststørrelsen) image Her er disse endringene til venstre og live til høyre, med 150% zoom (chrome)

Har du testet etter jeg endret til 1.2?

@tuva-odegard
Copy link
Contributor

Kan være noe Mac-greier, men det ser ikke ut som at det ble helt riktig hos meg.. Endringen førte til at knappen ble 5px lavere, og jeg får ikke samme resultatet som deg når jeg zoomer (øke tekststørrelsen) image Her er disse endringene til venstre og live til høyre, med 150% zoom (chrome)

Har du testet etter jeg endret til 1.2?

Kan teste etter den har blitt deployed, men det var 24px før, så blir ikke 1.2 litt for lite? Når jeg prøver i browseren ser det ut som det skal være 1.5 (16px * 1.5 = 24) i stedet

Copy link

github-actions bot commented Jul 2, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-2139.westeurope.2.azurestaticapps.net

@HeleneKassandra
Copy link
Contributor Author

Det ble veldig mye med 1.5 line-height. Det ble veldig stor avstand mellom linjene så 1.2 ser bedre ut. Men hvis knappene blir for små, så kan man kanskje kompensenere med mer padding?

@pethel
Copy link
Contributor

pethel commented Jul 2, 2024

Kan være noe Mac-greier, men det ser ikke ut som at det ble helt riktig hos meg.. Endringen førte til at knappen ble 5px lavere, og jeg får ikke samme resultatet som deg når jeg zoomer (øke tekststørrelsen) image Her er disse endringene til venstre og live til høyre, med 150% zoom (chrome)

Har du testet etter jeg endret til 1.2?

Kan teste etter den har blitt deployed, men det var 24px før, så blir ikke 1.2 litt for lite? Når jeg prøver i browseren ser det ut som det skal være 1.5 (16px * 1.5 = 24) i stedet

Du har rett att

Kan være noe Mac-greier, men det ser ikke ut som at det ble helt riktig hos meg.. Endringen førte til at knappen ble 5px lavere, og jeg får ikke samme resultatet som deg når jeg zoomer (øke tekststørrelsen) image Her er disse endringene til venstre og live til høyre, med 150% zoom (chrome)

Har du testet etter jeg endret til 1.2?

Kan teste etter den har blitt deployed, men det var 24px før, så blir ikke 1.2 litt for lite? Når jeg prøver i browseren ser det ut som det skal være 1.5 (16px * 1.5 = 24) i stedet

Det stemmer att vi går miste om lite højde her. Men det ser ikke bra ut med så stor lineheigth heller. Vi skall prova beholde lineheigten som er satt til 1.2 i denne Pr og heller kompensera med lite extra padding i top og bunn tenker jag.

@HeleneKassandra HeleneKassandra changed the title fix(ffe-buttons): endre line-height i basebutton til ch fix(ffe-buttons): endre line-height i basebutton Jul 2, 2024
@HeleneKassandra HeleneKassandra force-pushed the change-base-button-line-height-to-ch branch from cbb723f to be61436 Compare July 2, 2024 12:38
@HeleneKassandra
Copy link
Contributor Author

Endret paddingen litt, når jeg testet nå så ble den 44px men jeg har litt følelsen av at det endrer seg basert på font-størrelsen. Så litt usikker på hva som er best måte å sette padding på

Copy link

github-actions bot commented Jul 2, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-2139.westeurope.2.azurestaticapps.net

@HeleneKassandra HeleneKassandra force-pushed the change-base-button-line-height-to-ch branch from be61436 to b76b0fe Compare July 2, 2024 13:25
Copy link

github-actions bot commented Jul 2, 2024

Azure Static Web Apps: Your stage site is ready! Visit it here: https://black-beach-0d62d0d03-2139.westeurope.2.azurestaticapps.net

@pethel pethel merged commit b56d76f into develop Jul 2, 2024
3 checks passed
@pethel pethel deleted the change-base-button-line-height-to-ch branch July 2, 2024 13:31
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