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(components): [message-box] add autofocus attribute #8445

Merged
merged 5 commits into from Jun 25, 2022

Conversation

YunYouJun
Copy link
Member

@YunYouJun YunYouJun commented Jun 24, 2022

close #8213

  • add autofocus attribute (default: true)

The button in the message-box is no longer activated by default after opening, when set autofocus: false.

image


Please make sure these boxes are checked before submitting your PR, thank you!

  • Make sure you follow contributing guide English | (中文 | Español | Français).
  • Make sure you are merging your commits to dev branch.
  • Add some descriptions and refer to relative issues for your PR.

@github-actions
Copy link

github-actions bot commented Jun 24, 2022

@YunYouJun YunYouJun requested a review from a team June 24, 2022 04:00
@github-actions github-actions bot added the CommitMessage::Qualified Qualified commit message label Jun 24, 2022
Copy link
Collaborator

@sxzz sxzz left a comment

Choose a reason for hiding this comment

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

Maybe we should add this option to config provider too.

docs/examples/message-box/alert.vue Outdated Show resolved Hide resolved
@YunYouJun
Copy link
Member Author

Maybe we should add this option to config provider too.

It is possible to do this, but the message-box may be a little different.
The prompt auto-focuses input, which I think many people will want to turn on, while the default alert will want to turn off.

image

They may behave differently, so I think it's better to leave a dedicated property for this purpose.

@github-actions
Copy link

github-actions bot commented Jun 24, 2022

🧪 Playground Preview: https://element-plus.run/?pr=8445
Please comment the example via this playground if needed.

@tolking
Copy link
Member

tolking commented Jun 24, 2022

This change will break the original accessibility, Demo

@tolking
Copy link
Member

tolking commented Jun 24, 2022

Maybe we should not introduce this feature, remove the autofocus is a bad experience for some users.

Maybe we should guide developers who want to change it like this in the docs to change it through styles.

@YunYouJun
Copy link
Member Author

I got it. Fixed.

Copy link
Collaborator

@Giwayume Giwayume left a comment

Choose a reason for hiding this comment

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

In its current state, autofocus: false results in the close x being focused, which turns it blue. You may want to explicitly assign focusStartRef to rootRef when atuofocus is false. This will result in no visual change, and when the user tabs the focus will be trapped between the close and ok buttons.

@YunYouJun
Copy link
Member Author

@Giwayume Sorry I'm not familiar with a11y, I made some changes, Is this what you mean?

@tolking
Copy link
Member

tolking commented Jun 25, 2022

W3C specification

Focus is automatically set to the first focusable element inside the dialog

@Giwayume
Copy link
Collaborator

Giwayume commented Jun 25, 2022

@Giwayume Sorry I'm not familiar with a11y, I made some changes, Is this what you mean?

Yes, looks good!

@tolking

Those are examples for specific use cases, as far as focus and dialog goes it always depends on the use case for the dialog. There are cases where your dialog is purely informational and contains no real actions, in which case focus is pulled to the dialog element itself so its content is read out.

Anyhow, with the PR in its current state, autofocus defaults to true, which will behave the same as today.

If the developer changes autofocus to false, it will focus on the dialog root element instead which will trigger the dialog title to be read and possibly the full content depending on the screen reader. From there, the user can enter browse mode or tab to the buttons they need. It's not the best solution in terms of usability but it still meets accessibility requirements.

@YunYouJun YunYouJun merged commit 5d88f62 into dev Jun 25, 2022
@YunYouJun YunYouJun deleted the feat/message-box-autofocus branch June 25, 2022 20:32
@element-bot element-bot mentioned this pull request Jul 1, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CommitMessage::Qualified Qualified commit message
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request] ElMessageBox confirm button style
4 participants