Skip to content

Commit cad580d

Browse files
bmeurerDevtools-frontend LUCI CQ
authored andcommitted
[settings] Make (?) buttons consistent in settings.
A bunch of adjustments for the (?) buttons: 1. Consistently use "Learn more" as title. 2. When used in combination with a settings checkbox, and there's more information, this will be shown as tooltip when hovering the checkbox (rather than when hovering the (?) icon). 3. Consistently use the `pointer` cursor. Fixed: 374247106 Change-Id: Ia1e49de4e3e0ea72a17730b6935e358f89c84eaa Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5946885 Auto-Submit: Benedikt Meurer <[email protected]> Reviewed-by: Kateryna Prokopenko <[email protected]> Commit-Queue: Kateryna Prokopenko <[email protected]>
1 parent 51e8478 commit cad580d

File tree

7 files changed

+42
-15
lines changed

7 files changed

+42
-15
lines changed

front_end/panels/elements/ElementStatePaneWidget.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ const UIStrings = {
3838
* @description Similar with forceElementState but allows users to force specific state of the selected element.
3939
*/
4040
forceElementSpecificStates: 'Force specific element state',
41+
/**
42+
*@description Text that is usually a hyperlink to more documentation
43+
*/
44+
learnMore: 'Learn more',
4145
};
4246
const str_ = i18n.i18n.registerUIStrings('panels/elements/ElementStatePaneWidget.ts', UIStrings);
4347
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
@@ -125,6 +129,7 @@ export class ElementStatePaneWidget extends UI.Widget.Widget {
125129
iconName: 'help',
126130
size: Buttons.Button.Size.SMALL,
127131
jslogContext: 'learn-more',
132+
title: i18nString(UIStrings.learnMore),
128133
};
129134
learnMoreButton.addEventListener(
130135
'click',

front_end/panels/settings/SettingsScreen.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -496,10 +496,10 @@ export class ExperimentsSettingsTab extends SettingsTab {
496496
variant: Buttons.Button.Variant.ICON,
497497
size: Buttons.Button.Size.SMALL,
498498
jslogContext: `${experiment.name}-documentation`,
499+
title: i18nString(UIStrings.learnMore),
499500
};
500501
linkButton.addEventListener(
501502
'click', () => Host.InspectorFrontendHost.InspectorFrontendHostInstance.openInNewTab(experimentLink));
502-
linkButton.ariaLabel = i18nString(UIStrings.learnMore);
503503
linkButton.classList.add('link-icon');
504504

505505
p.appendChild(linkButton);

front_end/panels/settings/frameworkIgnoreListSettingsTab.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
}
1010

1111
.ignore-list-option devtools-button {
12+
cursor: pointer;
1213
position: relative;
1314
top: var(--sys-size-2);
1415
margin-left: var(--sys-size-2);

front_end/panels/settings/settingsScreen.css

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -213,15 +213,15 @@ label {
213213
.settings-experiment {
214214
display: grid;
215215
grid-template-columns: auto min-content auto 1fr;
216-
}
217216

218-
.settings-experiment .devtools-link {
219-
display: flex !important; /* stylelint-disable-line declaration-no-important */
220-
align-items: center;
217+
& .devtools-link {
218+
display: flex !important; /* stylelint-disable-line declaration-no-important */
219+
align-items: center;
220+
}
221221
}
222222

223-
.settings-experiment .devtools-link:has(.link-icon) {
224-
outline-offset: 0;
223+
devtools-button.link-icon {
224+
cursor: pointer;
225225
}
226226

227227
.experiment-label {

front_end/ui/components/buttons/button.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ button {
4040
align-items: center;
4141
background: transparent;
4242
border-radius: var(--sys-shape-corner-full);
43+
cursor: inherit;
4344
display: inline-flex;
4445
position: relative;
4546
font-size: var(--sys-typescale-body4-size);

front_end/ui/components/settings/SettingCheckbox.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import './SettingDeprecationWarning.js';
66

77
import type * as Common from '../../../core/common/common.js';
88
import * as Host from '../../../core/host/host.js';
9+
import * as i18n from '../../../core/i18n/i18n.js';
910
import * as LitHtml from '../../lit-html/lit-html.js';
1011
import * as VisualLogging from '../../visual_logging/visual_logging.js';
1112
import * as Buttons from '../buttons/buttons.js';
@@ -15,6 +16,15 @@ import settingCheckboxStyles from './settingCheckbox.css.js';
1516

1617
const {html, Directives: {ifDefined}} = LitHtml;
1718

19+
const UIStrings = {
20+
/**
21+
*@description Text that is usually a hyperlink to more documentation
22+
*/
23+
learnMore: 'Learn more',
24+
};
25+
const str_ = i18n.i18n.registerUIStrings('ui/components/settings/SettingCheckbox.ts', UIStrings);
26+
const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_);
27+
1828
export interface SettingCheckboxData {
1929
setting: Common.Settings.Setting<boolean>;
2030
textOverride?: string;
@@ -60,13 +70,21 @@ export class SettingCheckbox extends HTMLElement {
6070

6171
const learnMore = this.#setting.learnMore();
6272
if (learnMore) {
63-
const jslog = VisualLogging.link()
64-
.track({click: true, keydown: 'Enter|Space'})
65-
.context(this.#setting.name + '-documentation');
66-
return html`<devtools-button .iconName=${'help'} .size=${Buttons.Button.Size.SMALL} .variant=${
67-
Buttons.Button.Variant.ICON} .title=${learnMore.tooltip()} jslog=${jslog} @click=${
68-
() => Host.InspectorFrontendHost.InspectorFrontendHostInstance.openInNewTab(
69-
learnMore.url)} class="learn-more"></devtools-button>`;
73+
const data: Buttons.Button.ButtonData = {
74+
iconName: 'help',
75+
variant: Buttons.Button.Variant.ICON,
76+
size: Buttons.Button.Size.SMALL,
77+
jslogContext: `${this.#setting.name}-documentation`,
78+
title: i18nString(UIStrings.learnMore),
79+
};
80+
const handleClick = (event: MouseEvent): void => {
81+
Host.InspectorFrontendHost.InspectorFrontendHostInstance.openInNewTab(learnMore.url);
82+
event.consume();
83+
};
84+
return html`<devtools-button
85+
class=learn-more
86+
@click=${handleClick}
87+
.data=${data as Buttons.Button.ButtonData}></devtools-button>`;
7088
}
7189

7290
return undefined;
@@ -78,6 +96,7 @@ export class SettingCheckbox extends HTMLElement {
7896
}
7997

8098
const icon = this.icon();
99+
const title = this.#setting.learnMore() && this.#setting.learnMore()?.tooltip();
81100
const reason = this.#setting.disabledReason() ?
82101
html`
83102
<devtools-button class="disabled-reason" .iconName=${'info'} .variant=${Buttons.Button.Variant.ICON} .size=${
@@ -88,7 +107,7 @@ export class SettingCheckbox extends HTMLElement {
88107
LitHtml.render(
89108
html`
90109
<p>
91-
<label>
110+
<label title=${title ?? LitHtml.nothing}>
92111
<input
93112
type="checkbox"
94113
.checked=${this.#setting.disabledReason() ? false : this.#setting.get()}

front_end/ui/components/settings/settingCheckbox.css

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ p {
3636
}
3737

3838
.learn-more {
39+
cursor: pointer;
3940
position: relative;
4041
margin-left: var(--sys-size-2);
4142
top: var(--sys-size-2);

0 commit comments

Comments
 (0)