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
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions packages/commands/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,21 @@ class CommandRegistry {
return cmd ? cmd.isToggled.call(undefined, args) : false;
}

/**
* Test whether a specific command is toggleable.
*
* @param id - The id of the command of interest.
*
* @param args - The arguments for the command.
*
* @returns A boolean indicating whether the command is toggleable,
* or `false` if the command is not registered.
*/
isToggleable(id: string, args: ReadonlyJSONObject = JSONExt.emptyObject): boolean {
let cmd = this._commands[id];
return cmd ? cmd.isToggleable : false;
}

/**
* Test whether a specific command is visible.
*
Expand Down Expand Up @@ -759,6 +774,20 @@ namespace CommandRegistry {
*/
isToggled?: CommandFunc<boolean>;

/**
* A function which indicates whether the command is toggleable.
*
* #### Notes
* Visual representations may use this value to display a toggled command in
* a different form, such as a check box for a menu item or a depressed
* state for a toggle button. This attribute also allows for accessible
* interfaces to notify the user that the command corresponds to some state.
*
* The default value is `true` if an `isToggled` function is given, `false`
* otherwise.
*/
isToggleable?: boolean;

/**
* A function which indicates whether the command is visible.
*
Expand Down Expand Up @@ -1147,6 +1176,7 @@ namespace Private {
readonly dataset: CommandFunc<Dataset>;
readonly isEnabled: CommandFunc<boolean>;
readonly isToggled: CommandFunc<boolean>;
readonly isToggleable: boolean;
readonly isVisible: CommandFunc<boolean>;
}

Expand All @@ -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.

isVisible: options.isVisible || trueFunc
};
}
Expand Down