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

Toolbar items should be enabled/visible according to when-clause-contexts #11705

Closed
ndoschek opened this issue Sep 28, 2022 · 4 comments · Fixed by #12067
Closed

Toolbar items should be enabled/visible according to when-clause-contexts #11705

ndoschek opened this issue Sep 28, 2022 · 4 comments · Fixed by #12067
Assignees
Labels
toolbar issues related to the toolbar

Comments

@ndoschek
Copy link
Contributor

ndoschek commented Sep 28, 2022

Feature Description:

The TabBarToolbarItem interface for items for the the main toolbar offers an optional when property, that is supposed to accept when-clause-contexts.
This can be set, but it has no impact on the toolbar item.
Also, editing the user's toolbar.json directly in the application does not allow to set such a clause:
image

I would like to see items to be enabled/visible only if the when clause returns true (e.g. only show item if the workbench has a valid workspace: when: 'workbenchState != empty').

@colin-grant-work colin-grant-work added the toolbar issues related to the toolbar label Sep 28, 2022
@colin-grant-work
Copy link
Contributor

I would like to see items to be enabled/visible only if the when clause returns true

Would you prefer that the toolbar item be invisible, or visible but marked inactive?

@colin-grant-work
Copy link
Contributor

For the interested implementer, the points of reference are these:

protected override renderItem(
item: TabBarToolbarItem,
): React.ReactNode {
const classNames = [];
if (item.text) {
for (const labelPart of this.labelParser.parse(item.text)) {
if (typeof labelPart !== 'string' && LabelIcon.is(labelPart)) {
const className = `fa fa-${labelPart.name}${labelPart.animation ? ' fa-' + labelPart.animation : ''}`;
classNames.push(...className.split(' '));
}
}
}
const command = this.commands.getCommand(item.command);
const iconClass = (typeof item.icon === 'function' && item.icon()) || item.icon || command?.iconClass;
if (iconClass) {
classNames.push(iconClass);
}
let itemTooltip = '';
if (item.tooltip) {
itemTooltip = item.tooltip;
} else if (command?.label) {
itemTooltip = command.label;
}
const keybindingString = this.resolveKeybindingForCommand(command?.id);
itemTooltip = `${itemTooltip}${keybindingString}`;
return (
<div
id={item.id}
className={classNames.join(' ')}
title={itemTooltip}
/>
);
}

protected renderItem(item: AnyToolbarItem): React.ReactNode {
let innerText = '';
const classNames = [];
if (item.text) {
for (const labelPart of this.labelParser.parse(item.text)) {
if (typeof labelPart !== 'string' && LabelIcon.is(labelPart)) {
const className = `fa fa-${labelPart.name}${labelPart.animation ? ' fa-' + labelPart.animation : ''}`;
classNames.push(...className.split(' '));
} else {
innerText = labelPart;
}
}
}
const command = item.command ? this.commands.getCommand(item.command) : undefined;
let iconClass = (typeof item.icon === 'function' && item.icon()) || item.icon as string || (command && command.iconClass);
if (iconClass) {
iconClass += ` ${ACTION_ITEM}`;
classNames.push(iconClass);
}
const tooltip = item.tooltip || (command && command.label);
const toolbarItemClassNames = this.getToolbarItemClassNames(command?.id ?? item.command);
if (item.menuPath && !item.command) { toolbarItemClassNames.push('enabled'); }
return <div key={item.id}
ref={this.onRender}
className={toolbarItemClassNames.join(' ')}
onMouseDown={this.onMouseDownEvent}
onMouseUp={this.onMouseUpEvent}
onMouseOut={this.onMouseUpEvent} >
<div id={item.id} className={classNames.join(' ')}
onClick={this.executeCommand}
title={tooltip}>{innerText}
</div>
</div>;
}

@ndoschek
Copy link
Contributor Author

@colin-grant-work Thanks for your feedback!
I don't have a strong opinion, but looking at toolbars in other IDEs (VSCode, Eclipse RCP) items are mostly disabled (visible but inactive) if not relevant for the current context. So I'd suggest to go that way as well. WDYT?

@jonah-iden
Copy link
Contributor

I would like to work on this. Could someone assign me?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
toolbar issues related to the toolbar
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants