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: Add horizontal and vertical alignment options to Row #1330

Merged
merged 12 commits into from
Jul 19, 2024

Conversation

VincentSmedinga
Copy link
Contributor

@VincentSmedinga VincentSmedinga commented Jul 8, 2024

Rows can now align their items horizontally and vertically.

This is useful for:

  • right-aligning a single item, e.g. <Row align="end"><Avatar /></Row>
  • a link at the far end of a heading, e.g. <Row align="between"><Heading /><Link /></Row>
  • ‘between’ can also be used for ‘previous’ and ‘next‘ buttons if we want them like that
  • vertically aligning differently sized texts: <Row alignVertical="baseline"><Heading /><Paragraph /></Row>

After merging, I’ll do the same for Column.

This touches on an important API choice: Row has alignment and vertical alignment, Column has alignment and horizontal alignment. For both, the regular alignment works in their main direction. Physical directions is conceptually easier to understand than separating ‘justification’ and ‘alignment’ as the flexbox spec does. For CSS, that is the better API. For our DSL, horizontal and vertical is. We translate between them for the best of both worlds.

Horizontal alignment uses ‘start’ and ‘end’ as labels for the extremes; vertical uses ‘top’ and ‘bottom’. Again, these words are easy to guess (as soon as people are used to use ‘start’ instead of ‘left’, that is).

packages/css/src/components/row/row.scss Outdated Show resolved Hide resolved
packages/react/src/Row/Row.tsx Show resolved Hide resolved
packages/react/src/Row/Row.tsx Show resolved Hide resolved
packages/css/src/components/row/row.scss Outdated Show resolved Hide resolved
packages/css/src/components/row/row.scss Outdated Show resolved Hide resolved
storybook/src/components/Row/Row.docs.mdx Outdated Show resolved Hide resolved
storybook/src/components/Row/Row.stories.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@RubenSibon RubenSibon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that I agree with the rational to differ from how CSS handles alignment versus justification. Why not use justify instead of align if that is what web developers already are used to? And why not educate the users to think in terms of start and end for the "vertical" axis and locale agnostic directionality as Aram suggested?

@VincentSmedinga
Copy link
Contributor Author

I'm not sure that I agree with the rational to differ from how CSS handles alignment versus justification. Why not use justify instead of align if that is what web developers already are used to? And why not educate the users to think in terms of start and end for the "vertical" axis and locale agnostic directionality as Aram suggested?

I understand. See my replies to those comments. I want to be inclusive for designers and other roles in digital product development. Design systems are multidisciplinary. We as front-end devs are the ones structuring the design for everyonw, but the names we choose are not just for ourselves but the entire team. We feedback our iteration on modeling and naming to designers in order to reach common understanding. Development will always have some “oh that’s too technical, I don’t understand and don’t want to try” about it, and most things we do are best left behind our curtains, but I believe the shared concepts in the design should be easily shared, through common language.

@github-actions github-actions bot temporarily deployed to demo-row-alignment July 16, 2024 17:44 Destroyed
@github-actions github-actions bot temporarily deployed to demo-row-alignment July 17, 2024 18:30 Destroyed
@github-actions github-actions bot temporarily deployed to demo-row-alignment July 17, 2024 20:05 Destroyed
@github-actions github-actions bot temporarily deployed to demo-row-alignment July 18, 2024 09:18 Destroyed
Copy link
Contributor

@dlnr dlnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rest seems fine. I do wonder how we would deal with elements that use align-self, but if they do they are not added by us.

packages/react/src/index.ts Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to demo-row-alignment July 19, 2024 08:54 Destroyed
@alimpens alimpens merged commit 1b9c269 into develop Jul 19, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants