Skip to content
This repository has been archived by the owner on Nov 6, 2019. It is now read-only.

Add isToggleable command state #404

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jasongrout
Copy link
Member

This allows interfaces (including accessible ones) to indicate to the user that the command may toggle state.

This allows interfaces (including accessible ones) to indicate to the user that the command may toggle state.
@jasongrout
Copy link
Member Author

Looks like the test failure is from Travis, not from this PR.

@@ -1167,6 +1197,7 @@ namespace Private {
dataset: asFunc(options.dataset, emptyDatasetFunc),
isEnabled: options.isEnabled || trueFunc,
isToggled: options.isToggled || falseFunc,
isToggleable: options.isToggleable || !!options.isToggled,
Copy link
Member Author

Choose a reason for hiding this comment

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

In #392, @afshin commented

Is this correct? It seems like it should only be !!options.isToggled and never use the options.isToggleable.

The primary UX hint we had in mind was something that was toggleable should be represented in a menu as a checkbox (rather than just a UI element that visually was a check) so that screenreaders could recognized the toggled status. It sounds like you are basically saying that isToggleable should distinguish between the default isToggled and a user-supplied isToggled. I think you are also implying that there never is a case where the user supplies an isToggled function, but explicitly wants to say that the element is not toggleable.

I think your suggestion makes sense. If isToggleable really is just asking if the user supplied an isToggled function vs using the default function, there may be a better way to do it once we can break backwards compatibility.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant