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: fluid space scale #786

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

feat: fluid space scale #786

wants to merge 11 commits into from

Conversation

Robbert
Copy link
Member

@Robbert Robbert commented Nov 4, 2024

Pull Request

In deze PR heb ik de basis spacing scale toegevoegd aan de Voorbeeld thema

Basis spacing scale xx-small t/m xxxx-large. Scale values kloppen met de values van het bepaalde thema. Value refrences naam verwerkt naar de correcte refrence bijv.

voorbeeld.space.inline.snailbasis.space.inline.large

Issues
Er zijn nog een paar waarden die geen basis-thema waarde hebben bijv...

  "step-meta": {
          "padding-inline-start": {
            "$type": "spacing",
            "$value": "{voorbeeld.space.inline.pig}"
          }

Vragen

  1. Waarom gebruiken we nu de referentienaam xxxxxx-large in plaats van 6xlg? Voor de mensen die dit aanpassen of er gewoon naar kijken, is het lastig en onduidelijk door al die x-en.

  2. In het basis thema Figma bestaat er ook text spacing. Waarom passen we die niet aan?

  3. Jeffrey en ik hebben het Figma bestand van het basis thema aangepast naar de sizing van mijn token bestand, zodat alle waarden kloppen. De text spacing hebben we laten staan, en de naam is bijvoorbeeld nog sm in plaats van small. Moeten we deze ook aanpassen, zodat alles goed aligned is en consistent past?

  4. Bij het voorbeeld thema hebben de row en column verschillende space scales in vergelijking met inline en block. De row en column kunnen veel grotere afstanden hebben, die worden gebruikt. Aan de andere kant hebben 'inline' en 'block' kleinere spacings voor de boven- en onderkant van een stuk tekst bijv, maar bij de basis space scaling werd echter één scale gebruikt voor alle vier, wat het probleem veroorzaakte en waarom er extra values zijn zonder een basis thema scale. Is dit per ongeluk gebeurd, of is dit de bedoeling?

  5. Hoe gaan we de min- en max-scaling toepassen?

  6. De tokens leken nu bij Common te staan. Maar horen ze niet bij Brand?

  7. Nog even een checkvraag. Zou hierdoor alles altijd schalen dat spacing tokens heeft. Zijn er componenten waar je wilt dat ze altijd gelijk blijven? Of kan dit voorkomen worden door min en max? Dit gezegd hebbende kan ik zelf een componenten voorstellen niet niet zouden meeschalen (hence 'checkvraag')

@Robbert Robbert requested a review from a team as a code owner November 4, 2024 10:21
Copy link

vercel bot commented Nov 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
themes ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 6, 2024 11:35am

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.

2 participants