[docs] Add "reset focus" control to demo tools #20724
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
eps1lon
added
docs
Details of bundle changes.Comparing: 2231349...9bbd0bc Details of page changes
|
|
@eps1lon two questions:
|
That doesn't make much sense. It was there before because we didn't have an appropriate control. But the container itself doesn't hold any meaning beyond visually containing the demo. It's also counter-intuitive that tab focus moves inside a container when you want to focus the next thing. |
|
Let's try the outline. |
I remembered why I didn't want the focusable container. It can get accidentally focused on mousedown events on certain platforms. But we can prevent that by calling |
eps1lon
force-pushed
the
docs/demo-enhance
branch
from
9241c45 to
9bbd0bc
Compare
oliviertassinari
approved these changes
Apr 24, 2020
It's hard to tell what you mean by consistent if the outline isn't even consistent within the ration component. |
|
Here is what I mean, 4 different looks (color + shape):
|
|
|
That icon is less than ideal but I'd be surprised if there's an intuitive one for "reset focus". I've personally never seen this on documentation sites but feel like it's somewhat helpful for devs or auditors. Maybe someone finds a better one. |
| function handleDemoMouseDown(event) { | ||
| // Otherwise clicking any non-focusable element in the demo focuses the demo container | ||
| // which is surprising and not how the code would behave outside of this page | ||
| event.preventDefault(); |
There was a problem hiding this comment.
I have discovered a negative side effect. Developers will no longer be able to copy and paste from the demos. For instance, try copy and paste the output of the playground in your project (my use case for making a new demo with #20755)
https://master--material-ui.netlify.app/components/popover/#anchor-playground
=> we can't.
There was a problem hiding this comment.
Going back to the original implementation.
This was referenced Apr 29, 2020
This was referenced May 6, 2020
oliviertassinari
added a commit
to oliviertassinari/material-ui
that referenced
this pull request
May 10, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a programmatically focusable marker before the demo which can be focused via a button in the toolbar.
some goals: