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: Start-align Dialog buttons and place the primary button first #1143

Merged
merged 19 commits into from
May 17, 2024

Conversation

VincentSmedinga
Copy link
Contributor

@VincentSmedinga VincentSmedinga commented Mar 24, 2024

The conclusion of our research is to put the primary button first in a Dialog while still aligning them to the end of the line. See our ticket in Jira for Bas’ writeup.

I’ve implemented this with CSS that stacks the buttons vertically and stretches them if they don’t fit next to each other – so without a breakpoint 🎉

I documented the guideline as well.

@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment March 24, 2024 21:25 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment April 28, 2024 11:01 Destroyed
@VincentSmedinga
Copy link
Contributor Author

Previous description:

We’re still considering this; posting a PR to get a deployment to evaluate.

Even if we don’t follow through with the interaction design, we should merge the documentation improvements.

See also:

@VincentSmedinga VincentSmedinga marked this pull request as ready for review April 28, 2024 11:07
@VincentSmedinga VincentSmedinga changed the title feat: Align buttons in a Dialog to the start of the line chore: Add guideline to put the primary button first in a Dialog Apr 28, 2024
RubenSibon
RubenSibon previously approved these changes May 2, 2024
packages/css/src/components/dialog/README.md Outdated Show resolved Hide resolved
packages/css/src/components/dialog/README.md Outdated Show resolved Hide resolved
storybook/src/components/Dialog/Dialog.stories.tsx Outdated Show resolved Hide resolved
@alimpens
Copy link
Contributor

alimpens commented May 3, 2024

Hm, this feels a bit off for me because of the 'right is next' convention of LTR languages, but I don't know how strong that convention is.

NLS recommends aligning the submit button in a form to the left, to create a natural scan line. Isn't that also relevant here? A dialog is sort of form-like.

@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment May 6, 2024 17:46 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment May 6, 2024 17:55 Destroyed
@VincentSmedinga
Copy link
Contributor Author

Hm, this feels a bit off for me because of the 'right is next' convention of LTR languages, but I don't know how strong that convention is.

NLS recommends aligning the submit button in a form to the left, to create a natural scan line. Isn't that also relevant here? A dialog is sort of form-like.

You’re mixing up forms and dialogs here.

Forms in dialogs are against our design guidelines. We will only use dialogs for simple confirmations, warnings or information. They will either have 1 button ‘I understand’, or 2, coming down to ‘Yes’ and ‘No’. In these contexts, having the primary action as the first one is logical when reading, listening and focusing.

Forms or other flows with multiple steps will be on a regular page and have their own rules for button placement – basically ‘Previous’ aligned to the left and ‘Next’ to the right.

@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment May 6, 2024 18:12 Destroyed
@alimpens
Copy link
Contributor

alimpens commented May 7, 2024

You’re mixing up forms and dialogs here.

No, not really. I'm just saying the guidelines on forms seems relevant here as well. Having a single scan line and having the button more visible for users that zoom in seem relevant for this case too.

Forms or other flows with multiple steps will be on a regular page and have their own rules for button placement – basically ‘Previous’ aligned to the left and ‘Next’ to the right.

This probably won't be the button placement, based on the guidelines I linked. Both 'Previous' and 'Next' will be left-aligned. My point is that that same alignment makes sense here imo, and it also makes our overall button alignment more consistent.

@VincentSmedinga
Copy link
Contributor Author

You’re mixing up forms and dialogs here.

No, not really. I'm just saying the guidelines on forms seems relevant here as well. Having a single scan line and having the button more visible for users that zoom in seem relevant for this case too.

Sorry for my misunderstanding. What change do you suggest for this PR?

Forms or other flows with multiple steps will be on a regular page and have their own rules for button placement – basically ‘Previous’ aligned to the left and ‘Next’ to the right.

This probably won't be the button placement, based on the guidelines I linked. Both 'Previous' and 'Next' will be left-aligned. My point is that that same alignment makes sense here imo, and it also makes our overall button alignment more consistent.

That would be fine as well – what I really meant was that form button alignment is out of scope here; shouldn’t have mentioned the explicit example. Let’s discuss that in that future PR.

@alimpens
Copy link
Contributor

alimpens commented May 7, 2024

Sorry for my misunderstanding. What change do you suggest for this PR?

I suggest left-aligning the buttons. That way, the primary button is visually more 'anchored', and it has the 2 benefits mentioned in the NLDS guidelines on button placement in forms.

@VincentSmedinga
Copy link
Contributor Author

VincentSmedinga commented May 7, 2024

I suggest left-aligning the buttons.

Ah! Only now I understand what you aimed for with your first comment.

Bas did consider this, but decided to follow e.g. this article and right-align buttons in dialog specifically. Have you read the accompanying ticket in Jira? Let’s follow up there or face to face :)

@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment May 13, 2024 05:08 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment May 14, 2024 07:59 Destroyed
@VincentSmedinga VincentSmedinga changed the title chore: Add guideline to put the primary button first in a Dialog feat: Start-align Dialog buttons and place the primary button first May 14, 2024
@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment May 14, 2024 08:03 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment May 16, 2024 09:44 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment May 17, 2024 21:53 Destroyed
@github-actions github-actions bot temporarily deployed to demo-DES-553-dialog-buttons-alignment May 17, 2024 21:57 Destroyed
@VincentSmedinga VincentSmedinga merged commit fd668c1 into develop May 17, 2024
4 checks passed
@VincentSmedinga VincentSmedinga deleted the feature/DES-553-dialog-buttons-alignment branch May 17, 2024 22:00
@github-actions github-actions bot mentioned this pull request Jun 5, 2024
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