question non-blocking: GlAvatar prop shape accepts 'circle' or 'rect' values. Should square be rect or do we progress with square and retrofit component to align? What does the Figma component use? Should we align naming here?

suggestion non-blocking: can we escape this by removing square in design token defintions altogether?:

avatar
  border
    radius
      default
      lg
  circle
    border
      radius
        default

Now, I've gone full circle back to:

avatar
  border
    radius
      default
      lg
      circle

Thanks @danmh! Aligned on approach 🤝 Just a couple suggestions 🏓

I've spent some time wrestling with it, but I don't think we can.

I agree, let's progress with what you have in this MR 🤝

thought non-blocking: I'd like to play around with how we might define width/height/size tokens for the size scale and how they might co-ordinate with radius tokens:

avatar
  16
    width
    height
    font-size
  24
    width
    height
    font-size
  32
    width
    height
    font-size
  48
    width
    height
    font-size
  64
    width
    height
    font-size
  96
    width
    height
    font-size
  circle
    border
      radius
        default
  square
    border
      radius
        default
        lg

This might be enough tbh, since it allows size and shape to compliment each-other and be multiplicative. Ignore me, thanks for coming to my ted-talk.

I want to communicate why I raised this for prosperity to remind myself later. I don't think these are immediate concerns but are top of mind when I'm thinking about design token definitions:

• Single-level scales are easier are iterable and easier to automate for Tailwind utilities, design token visualisations/tables etc. When we multi-level it becomes tricker and requires more manual intervention in tooling. This is less of a concern for contextual design tokens which (currently) do no require Tailwind utility automations.
• Using the sizes as a scale could enable other properties like width/height to be siblings in the same definitions. Moving design decisions from component stylesheets to design tokens. However, there's far more complexity here with mixins and media queries which makes this approach untenable.
• Baking in circle and square bakes in visual styles to names which infer how something will looks. This is beneficial for prescribing meaning to tokens, but means they're less flexible to visual change. For example, everything circle or hexagonal becomes trendy and is adopted these have less meaning. This is less of a concern as we use shape to differentiate between projects/groups/users avatars so intervention here is greater than visual design trend.

suggestion: apply design token

  border-radius: var(--gl-avatar-circle-border-radius-default);

@GitLabDuo I appreciate the defensive CSS here, however if any CSS custom property for design tokens isn't defined in consumers, we're in for a much bigger world of hurt then just this property 😂 Please note that all consumers of GitLab UI inherit all design tokens as CSS custom properties on the :root element. They should be available for any consumer of gitlab-ui components.

Cool, this sounds like a sensible approach then. I'd bet that the larger radius is intentional and should be shifted-left to the avatar component anyway 👍

note: I updated the description here to be more explicit about create a Figma branch on "Design tokens" Figma file and switching to /packages/gitlab-ui directory to run script.

Scott de Jonge (4493bc6f) at 18 Mar 22:27

suggestion: robot is no longer required now it's part of a dangerbot comment anyway, use originally proposed "Figma Token Sync Reminder" language to soften message

    ## Figma Token Sync Reminder

Thanks for this example! Closing in favour of !5755 🙇

What does this MR do?

Reimplements !5755 using the existing Danger setup instead of a custom CI job.

Whenever *.tokens.json files are modified, a Danger warning and markdown comment are posted on the MR reminding the author to run yarn sync-tokens and sync the Figma "Design tokens" library.

Why Danger instead of a custom CI job?

• No new CI job, no dedicated API token (GITLAB_CANONICAL_INTEGRATION_REST_TOKEN), and no Node script needed
• Duplicate-comment deduplication is handled automatically by Danger (it edits its single sticky comment rather than posting new ones)
• Follows the same pattern already used by danger/gitlab_ui_specs and danger/simple_ux_review
• import_dangerfiles in the root Dangerfile auto-discovers danger/sync_tokens/Dangerfile — no root Dangerfile changes required

Related to #3360

Awwww yeah! @markrian that's much simplier/cleaner/nicer thank you!

Though, the danger comment doesn't create a thread which needs resolving, which this approach does. Is that essential?

Nah, the ping for me to run script is the immediate need, in Danger is nicer because it's less disruptive to other contributors. This is a task we (me) need to refine and automate anyway.

I/Duo created !5759 (closed) as a replacement. Did you receive the ping from !5759 (comment 3169946359)? WDYT?

Lovely jubbley! I've brought the Dangerfile change into this branch to allow you to review/merge, thanks!

Scott de Jonge (4b2ed518) at 18 Mar 22:24

Close as this is just a test

What does this MR do?

This is a test

Screenshots or screen recordings

This is a test

Integrations

This is a test

Does this MR meet the acceptance criteria?

This checklist encourages the authors, reviewers, and maintainers of merge requests (MRs) to confirm changes were analyzed for conformity with the project's guidelines, security and accessibility.

Related to #3360

is Storybook something we directly want to promote contributing to as part of this section?

@jeldergl maybe not, but until we have a suitable solution which covers majority current stories it's going to be resource used by engineering for component/prop/slot discovery. I think this is a minimal intervention way to provide a link for those looking for it which can be refined at a later date once we have alternatives. I don't hold this strongly, and happy to take the view of others. I'd like to progress and iterate to satisfy the immediate consumer need raised in Slack, wdyt?

What does this MR do?

• Move body reset styles to body.scss
• Add font.family.base and line-height.base design tokens
• Move Bootstrap _reboot.scss to reset.scss

Screenshots or screen recordings

n/a

Integrations

• GitLab: mr_url
• CustomersDot: mr_url
• Duo UI: mr_url
• Status Page: mr_url
• Docs: mr_url
• Switchboard: mr_url

Does this MR meet the acceptance criteria?

This checklist encourages the authors, reviewers, and maintainers of merge requests (MRs) to confirm changes were analyzed for conformity with the project's guidelines, security and accessibility.

Related to #3358

Scott de Jonge (8573409a) at 18 Mar 06:17

... and 2 more commits