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

feat: oppdater card til å matche nyeste versjon i Figma #4079

Merged
merged 2 commits into from
Sep 16, 2024

Conversation

piofinn
Copy link
Contributor

@piofinn piofinn commented Sep 10, 2024

Justerer antall bakgrunnsfarger og spacinger til å matche oppdatert komponent i Figma

BREAKING CHANGE:
Fargene "inverted" og "subdued", samt spacingene "none" og "xs", er fjernet

ISSUES CLOSED: #4078

🎯 Sjekkliste

@piofinn piofinn self-assigned this Sep 10, 2024
@fremtind-bot
Copy link
Collaborator

fremtind-bot commented Sep 10, 2024

Forhåndsvisning: https://jokul.fremtind.no/preview/feat/card-update/
🔍 Commit: a29a569

Forhåndsvisningen blir tilgjengelig innen et par minutter. Den fjernes automatisk når pull requesten lukkes.

fremtind-bot added a commit that referenced this pull request Sep 10, 2024
@Stormoen
Copy link

Stormoen commented Sep 11, 2024

Ser bra ut. Noen kommentarer:

  • synes vi bør endre oppsettet i eksempelet - oppfordre til en mer vertikal layout enn "2x2" grid. Jeg kan sette opp noe.
  • Background "default" er lik som bakgrunnsfargen i vinduet. Usikker på hvem som er feil.
  • Egentlig bør kanskje default i komponenten være high? Bør vi da se på et annet navn for "default", eller kanskje vi bare skal tilby low og high, evt inverted likevel? Ikke så god kontrast til "background page/variant" med den som er default nå. - Klikkbart card LOW, altså med skygge ser ikke bra ut. hmm.

@piofinn
Copy link
Contributor Author

piofinn commented Sep 11, 2024

@Stormoen Har justert per endringene vi diskuterte tidligere 😊

high er satt som default type i komponenten, men jeg satte outlined som valgt i eksemplene pga. fargene som er brukt i portalen. Vi må se litt på kontrast mellom de forskjellige bakgrunnsfargene når vi jobber videre med nytt fargesystem; det er f.eks. veldig lav kontrast mellom container-high og page-variant i dagens system.

Edit: Jeg lurer også litt på om vi burde bytte navn på egenskapen "Type" til "Variant" (eller noe annet). Ordet "type" har en veldig spesifikk betydning i kode, så det kan potensielt virke litt unaturlig og forvirrende. Jeg liker "variant", og det er også brukt i mange andre komponenter, men kom gjerne med forslag! "Style" funker dessverre heller ikke, da det allerede er en egenskap som kan settes på alle web-elementer.

fremtind-bot added a commit that referenced this pull request Sep 11, 2024
fremtind-bot added a commit that referenced this pull request Sep 11, 2024
ivarni
ivarni previously approved these changes Sep 12, 2024
Justerer antall bakgrunnsfarger og spacinger til å matche
oppdatert komponent i Figma

BREAKING CHANGE:
Fargene "inverted" og "subdued", samt spacingene "none"
og "xs", er fjernet

ISSUES CLOSED: #4078
BREAKING CHANGE: Prop-ene "background" og "type" er fjernet. Du bestemmer nå
utseendet til kortet med prop-en "variant", som tar inn verdiene
"outlined", "high" og "low".
fremtind-bot added a commit that referenced this pull request Sep 13, 2024
fremtind-bot added a commit that referenced this pull request Sep 13, 2024
@piofinn piofinn added this pull request to the merge queue Sep 16, 2024
Merged via the queue into main with commit 472d693 Sep 16, 2024
5 checks passed
@piofinn piofinn deleted the feat/card-update branch September 16, 2024 12:27
github-actions bot pushed a commit that referenced this pull request Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Oppdatere Card sin kode til å matche nye skisser
4 participants