Skip to content

Commit 8e5043e

Browse files
wolfibDevtools-frontend LUCI CQ
authored andcommitted
[AI Settings] Show multiple disable-reasons
Screenshot: https://i.imgur.com/6BvvIdF.png Bug: 381127660 Change-Id: I09f9254712df8ab786501cba88ccb1e8e1231f10 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6097498 Commit-Queue: Wolfgang Beyer <[email protected]> Reviewed-by: Alex Rudenko <[email protected]> Commit-Queue: Alex Rudenko <[email protected]> Auto-Submit: Wolfgang Beyer <[email protected]>
1 parent 0372032 commit 8e5043e

File tree

10 files changed

+92
-64
lines changed

10 files changed

+92
-64
lines changed

front_end/core/common/SettingRegistration.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,5 +330,5 @@ interface RawSettingExtensionOption {
330330
export type SettingExtensionOption = LocalizedSettingExtensionOption|RawSettingExtensionOption;
331331
export type DisabledConditionResult = {
332332
disabled: true,
333-
reason: string,
333+
reasons: string[],
334334
}|{disabled: false};

front_end/core/common/Settings.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -431,14 +431,14 @@ export class Setting<V> {
431431
return this.#disabled || false;
432432
}
433433

434-
disabledReason(): string|undefined {
434+
disabledReasons(): string[] {
435435
if (this.#registration?.disabledCondition) {
436436
const result = this.#registration.disabledCondition(Settings.instance().getHostConfig());
437437
if (result.disabled) {
438-
return result.reason;
438+
return result.reasons;
439439
}
440440
}
441-
return undefined;
441+
return [];
442442
}
443443

444444
setDisabled(disabled: boolean): void {

front_end/panels/ai_assistance/ai_assistance-meta.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,17 @@ const UIStrings = {
3737
* @description Message shown to the user if the DevTools locale is not
3838
* supported.
3939
*/
40-
wrongLocale: 'To use this feature, set your language preference to English in DevTools settings',
40+
wrongLocale: 'To use this feature, set your language preference to English in DevTools settings.',
4141
/**
4242
* @description Message shown to the user if the user's region is not
4343
* supported.
4444
*/
45-
geoRestricted: 'This feature is unavailable in your region',
45+
geoRestricted: 'This feature is unavailable in your region.',
4646
/**
4747
* @description Message shown to the user if the enterprise policy does
4848
* not allow this feature.
4949
*/
50-
policyRestricted: 'This setting is managed by your administrator',
50+
policyRestricted: 'This setting is managed by your administrator.',
5151
};
5252

5353
const str_ = i18n.i18n.registerUIStrings('panels/ai_assistance/ai_assistance-meta.ts', UIStrings);
@@ -123,14 +123,18 @@ Common.Settings.registerSettingExtension({
123123
reloadRequired: false,
124124
condition: isAnyFeatureAvailable,
125125
disabledCondition: config => {
126+
const reasons = [];
126127
if (isGeoRestricted(config)) {
127-
return {disabled: true, reason: i18nString(UIStrings.geoRestricted)};
128+
reasons.push(i18nString(UIStrings.geoRestricted));
128129
}
129130
if (isPolicyRestricted(config)) {
130-
return {disabled: true, reason: i18nString(UIStrings.policyRestricted)};
131+
reasons.push(i18nString(UIStrings.policyRestricted));
131132
}
132133
if (isLocaleRestricted()) {
133-
return {disabled: true, reason: i18nString(UIStrings.wrongLocale)};
134+
reasons.push(i18nString(UIStrings.wrongLocale));
135+
}
136+
if (reasons.length > 0) {
137+
return {disabled: true, reasons};
134138
}
135139
return {disabled: false};
136140
},

front_end/panels/explain/components/ConsoleInsight.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ describeWithEnvironment('ConsoleInsight', () => {
122122
settingType: Common.Settings.SettingType.BOOLEAN,
123123
defaultValue: true,
124124
disabledCondition: () => {
125-
return {disabled: true, reason: 'disabled for test'};
125+
return {disabled: true, reasons: ['disabled for test']};
126126
},
127127
});
128128
component = new Explain.ConsoleInsight(

front_end/panels/explain/explain-meta.ts

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,17 +30,17 @@ const UIStrings = {
3030
* @description Message shown to the user if the DevTools locale is not
3131
* supported.
3232
*/
33-
wrongLocale: 'To use this feature, set your language preference to English in DevTools settings',
33+
wrongLocale: 'To use this feature, set your language preference to English in DevTools settings.',
3434
/**
3535
* @description Message shown to the user if the user's region is not
3636
* supported.
3737
*/
38-
geoRestricted: 'This feature is unavailable in your region',
38+
geoRestricted: 'This feature is unavailable in your region.',
3939
/**
4040
* @description Message shown to the user if the enterprise policy does
4141
* not allow this feature.
4242
*/
43-
policyRestricted: 'This setting is managed by your administrator',
43+
policyRestricted: 'This setting is managed by your administrator.',
4444
};
4545
const str_ = i18n.i18n.registerUIStrings('panels/explain/explain-meta.ts', UIStrings);
4646
const i18nLazyString = i18n.i18n.getLazilyComputedLocalizedString.bind(undefined, str_);
@@ -105,14 +105,18 @@ Common.Settings.registerSettingExtension({
105105
reloadRequired: false,
106106
condition: config => isFeatureEnabled(config),
107107
disabledCondition: config => {
108+
const reasons = [];
108109
if (isGeoRestricted(config)) {
109-
return {disabled: true, reason: i18nString(UIStrings.geoRestricted)};
110+
reasons.push(i18nString(UIStrings.geoRestricted));
110111
}
111112
if (isPolicyRestricted(config)) {
112-
return {disabled: true, reason: i18nString(UIStrings.policyRestricted)};
113+
reasons.push(i18nString(UIStrings.policyRestricted));
113114
}
114115
if (isLocaleRestricted()) {
115-
return {disabled: true, reason: i18nString(UIStrings.wrongLocale)};
116+
reasons.push(i18nString(UIStrings.wrongLocale));
117+
}
118+
if (reasons.length > 0) {
119+
return {disabled: true, reasons};
116120
}
117121
return {disabled: false};
118122
},

front_end/panels/settings/AISettingsTab.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ describeWithEnvironment('AISettingsTab', () => {
168168
});
169169

170170
it('disables switches if blocked by age', async () => {
171-
const underAgeExplainer = 'This feature is only available to users who are 18 years of age or older';
171+
const underAgeExplainer = 'This feature is only available to users who are 18 years of age or older.';
172172
const aidaAccessStub = sinon.stub(Host.AidaClient.AidaClient, 'checkAccessPreconditions');
173173
aidaAccessStub.returns(Promise.resolve(Host.AidaClient.AidaAccessPreconditions.AVAILABLE));
174174
const hostConfigStub = getGetHostConfigStub({
@@ -223,15 +223,15 @@ describeWithEnvironment('AISettingsTab', () => {
223223
settingType: Common.Settings.SettingType.BOOLEAN,
224224
defaultValue: false,
225225
disabledCondition: () => {
226-
return {disabled: true, reason: 'some reason'};
226+
return {disabled: true, reasons: ['some reason']};
227227
},
228228
});
229229
Common.Settings.moduleSetting('ai-assistance-enabled').setRegistration({
230230
settingName: 'ai-assistance-enabled',
231231
settingType: Common.Settings.SettingType.BOOLEAN,
232232
defaultValue: true,
233233
disabledCondition: () => {
234-
return {disabled: true, reason: 'some reason'};
234+
return {disabled: true, reasons: ['some reason']};
235235
},
236236
});
237237
const stub = sinon.stub(Host.AidaClient.AidaClient, 'checkAccessPreconditions');

front_end/panels/settings/AISettingsTab.ts

Lines changed: 50 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ const UIStrings = {
154154
/**
155155
* @description Message shown to the user if the age check is not successful.
156156
*/
157-
ageRestricted: 'This feature is only available to users who are 18 years of age or older',
157+
ageRestricted: 'This feature is only available to users who are 18 years of age or older.',
158158
/**
159159
* @description The error message when the user is not logged in into Chrome.
160160
*/
@@ -378,24 +378,33 @@ export class AISettingsTab extends LegacyWrapper.LegacyWrapper.WrappableComponen
378378
// clang-format on
379379
}
380380

381-
#getDisabledReason(): string|undefined {
381+
#getDisabledReasons(): string[] {
382+
const reasons = [];
382383
switch (this.#aidaAvailability) {
383384
case Host.AidaClient.AidaAccessPreconditions.NO_ACCOUNT_EMAIL:
384385
case Host.AidaClient.AidaAccessPreconditions.SYNC_IS_PAUSED:
385-
return i18nString(UIStrings.notLoggedIn);
386-
case Host.AidaClient.AidaAccessPreconditions.NO_INTERNET:
387-
return i18nString(UIStrings.offline);
386+
reasons.push(i18nString(UIStrings.notLoggedIn));
387+
break;
388+
// @ts-expect-error
389+
case Host.AidaClient.AidaAccessPreconditions.NO_INTERNET: // fallthrough
390+
reasons.push(i18nString(UIStrings.offline));
391+
case Host.AidaClient.AidaAccessPreconditions.AVAILABLE: {
392+
// No age check if there is no logged in user. Age check would always fail in that case.
393+
const config = Common.Settings.Settings.instance().getHostConfig();
394+
if (config?.aidaAvailability?.blockedByAge === true) {
395+
reasons.push(i18nString(UIStrings.ageRestricted));
396+
}
397+
}
388398
}
389-
390-
const config = Common.Settings.Settings.instance().getHostConfig();
391-
if (config?.aidaAvailability?.blockedByAge === true) {
392-
return i18nString(UIStrings.ageRestricted);
393-
}
394-
// `consoleInsightsSetting` and `aiAssistantSetting` are both disabled for the same reason.
395-
return this.#consoleInsightsSetting?.disabledReason();
399+
// `consoleInsightsSetting` and `aiAssistantSetting` are both disabled for the same reasons.
400+
const disabledReasons = this.#consoleInsightsSetting?.disabledReasons() || [];
401+
reasons.push(...disabledReasons);
402+
return reasons;
396403
}
397404

398-
#renderConsoleInsightsSetting(disabledReason?: string): LitHtml.TemplateResult {
405+
#renderConsoleInsightsSetting(disabledReasons: string[]): LitHtml.TemplateResult {
406+
const isDisabled = disabledReasons.length > 0;
407+
const disabledReasonsJoined = disabledReasons.join('\n') || undefined;
399408
const detailsClasses = {
400409
'whole-row': true,
401410
open: this.#isConsoleInsightsSettingExpanded,
@@ -427,15 +436,15 @@ export class AISettingsTab extends LegacyWrapper.LegacyWrapper.WrappableComponen
427436
</div>
428437
<div class="divider"></div>
429438
<div class="toggle-container centered"
430-
title=${ifDefined(disabledReason)}
439+
title=${ifDefined(disabledReasonsJoined)}
431440
@click=${this.#toggleConsoleInsightsSetting.bind(this)}
432441
>
433442
<devtools-switch
434-
.checked=${Boolean(this.#consoleInsightsSetting?.get()) && !Boolean(disabledReason)}
443+
.checked=${Boolean(this.#consoleInsightsSetting?.get()) && !isDisabled}
435444
.jslogContext=${this.#consoleInsightsSetting?.name || ''}
436-
.disabled=${Boolean(disabledReason)}
445+
.disabled=${isDisabled}
437446
@switchchange=${this.#toggleConsoleInsightsSetting.bind(this)}
438-
aria-label=${disabledReason || i18nString(UIStrings.enableConsoleInsights)}
447+
aria-label=${disabledReasonsJoined || i18nString(UIStrings.enableConsoleInsights)}
439448
></devtools-switch>
440449
</div>
441450
<div class=${classMap(detailsClasses)}>
@@ -463,7 +472,9 @@ export class AISettingsTab extends LegacyWrapper.LegacyWrapper.WrappableComponen
463472
// clang-format on
464473
}
465474

466-
#renderAiAssistanceSetting(disabledReason?: string): LitHtml.TemplateResult {
475+
#renderAiAssistanceSetting(disabledReasons: string[]): LitHtml.TemplateResult {
476+
const isDisabled = disabledReasons.length > 0;
477+
const disabledReasonsJoined = disabledReasons.join('\n') || undefined;
467478
const detailsClasses = {
468479
'whole-row': true,
469480
open: this.#isAiAssistanceSettingExpanded,
@@ -495,15 +506,15 @@ export class AISettingsTab extends LegacyWrapper.LegacyWrapper.WrappableComponen
495506
</div>
496507
<div class="divider"></div>
497508
<div class="toggle-container centered"
498-
title=${ifDefined(disabledReason)}
509+
title=${ifDefined(disabledReasonsJoined)}
499510
@click=${this.#toggleAiAssistanceSetting.bind(this)}
500511
>
501512
<devtools-switch
502-
.checked=${Boolean(this.#aiAssistanceSetting?.get()) && !Boolean(disabledReason)}
513+
.checked=${Boolean(this.#aiAssistanceSetting?.get()) && !isDisabled}
503514
.jslogContext=${this.#aiAssistanceSetting?.name || ''}
504-
.disabled=${Boolean(disabledReason)}
515+
.disabled=${isDisabled}
505516
@switchchange=${this.#toggleAiAssistanceSetting.bind(this)}
506-
aria-label=${disabledReason || i18nString(UIStrings.enableAiAssistance)}
517+
aria-label=${disabledReasonsJoined || i18nString(UIStrings.enableAiAssistance)}
507518
></devtools-switch>
508519
</div>
509520
<div class=${classMap(detailsClasses)}>
@@ -531,37 +542,41 @@ export class AISettingsTab extends LegacyWrapper.LegacyWrapper.WrappableComponen
531542
// clang-format on
532543
}
533544

534-
#renderDisabledExplainer(disabledReason: string): LitHtml.LitTemplate {
545+
#renderDisabledExplainer(disabledReasons: string[]): LitHtml.LitTemplate {
535546
// Disabled until https://crbug.com/1079231 is fixed.
536547
// clang-format off
537548
return html`
538549
<div class="disabled-explainer">
539-
<devtools-icon .data=${{
540-
iconName: 'warning',
541-
color: 'var(--sys-color-orange)',
542-
width: 'var(--sys-size-8)',
543-
height: 'var(--sys-size-8)',
544-
} as IconButton.Icon.IconData}>
545-
</devtools-icon>
546-
${disabledReason}
550+
${disabledReasons.map(reason => html`
551+
<div class="disabled-explainer-row">
552+
<devtools-icon .data=${{
553+
iconName: 'warning',
554+
color: 'var(--sys-color-orange)',
555+
width: 'var(--sys-size-8)',
556+
height: 'var(--sys-size-8)',
557+
} as IconButton.Icon.IconData}>
558+
</devtools-icon>
559+
${reason}
560+
</div>
561+
`)}
547562
</div>
548563
`;
549564
// clang-format on
550565
}
551566

552567
override async render(): Promise<void> {
553-
const disabledReason = this.#getDisabledReason();
568+
const disabledReasons = this.#getDisabledReasons();
554569

555570
// Disabled until https://crbug.com/1079231 is fixed.
556571
// clang-format off
557572
LitHtml.render(html`
558573
<div class="settings-container-wrapper" jslog=${VisualLogging.pane('chrome-ai')}>
559574
${this.#renderSharedDisclaimer()}
560575
${this.#consoleInsightsSetting || this.#aiAssistanceSetting ? html`
561-
${Boolean(disabledReason) ? this.#renderDisabledExplainer(disabledReason as string) : LitHtml.nothing}
576+
${disabledReasons.length ? this.#renderDisabledExplainer(disabledReasons) : LitHtml.nothing}
562577
<div class="settings-container">
563-
${this.#consoleInsightsSetting ? this.#renderConsoleInsightsSetting(disabledReason) : LitHtml.nothing}
564-
${this.#aiAssistanceSetting ? this.#renderAiAssistanceSetting(disabledReason) : LitHtml.nothing}
578+
${this.#consoleInsightsSetting ? this.#renderConsoleInsightsSetting(disabledReasons) : LitHtml.nothing}
579+
${this.#aiAssistanceSetting ? this.#renderAiAssistanceSetting(disabledReasons) : LitHtml.nothing}
565580
</div>
566581
` : LitHtml.nothing}
567582
</div>

front_end/panels/settings/aiSettingsTab.css

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,14 +172,18 @@ header {
172172
}
173173

174174
.disabled-explainer {
175-
display: flex;
176-
gap: var(--sys-size-6);
177175
background-color: var(--sys-color-surface-yellow);
178176
border-radius: var(--sys-shape-corner-medium-small);
179177
margin-top: var(--sys-size-11);
180-
padding: var(--sys-size-8) var(--sys-size-11);
178+
padding: var(--sys-size-6) var(--sys-size-11) var(--sys-size-8);
181179
width: 100%;
182180
max-width: var(--sys-size-35);
183181
min-width: var(--sys-size-28);
184182
color: var(--sys-color-yellow);
185183
}
184+
185+
.disabled-explainer-row {
186+
display: flex;
187+
gap: var(--sys-size-6);
188+
margin-top: var(--sys-size-4);
189+
}

front_end/ui/components/settings/SettingCheckbox.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ describe('SettingCheckbox', () => {
5656
assert.isFalse(checkbox.checked);
5757
});
5858

59-
it('can be reassigned to a different settings', () => {
59+
it('can be reassigned to a different setting', () => {
6060
const setting1 = createFakeSetting<boolean>('setting1', true);
6161
const setting2 = createFakeSetting<boolean>('setting2', true);
6262
const {component, checkbox} = renderSettingCheckbox({setting: setting1});
@@ -101,7 +101,7 @@ describe('SettingCheckbox', () => {
101101
settingType: Common.Settings.SettingType.BOOLEAN,
102102
defaultValue: false,
103103
disabledCondition: () => {
104-
return {disabled: true, reason: 'reason'};
104+
return {disabled: true, reasons: ['reason']};
105105
},
106106
});
107107

@@ -121,7 +121,7 @@ describe('SettingCheckbox', () => {
121121
settingType: Common.Settings.SettingType.BOOLEAN,
122122
defaultValue: false,
123123
disabledCondition: () => {
124-
return {disabled: true, reason: 'reason'};
124+
return {disabled: true, reasons: ['reason']};
125125
},
126126
});
127127

front_end/ui/components/settings/SettingCheckbox.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,11 @@ export class SettingCheckbox extends HTMLElement {
9898

9999
const icon = this.icon();
100100
const title = `${this.#setting.learnMore() ? this.#setting.learnMore()?.tooltip() : ''}`;
101-
const reason = this.#setting.disabledReason() ?
101+
const disabledReasons = this.#setting.disabledReasons();
102+
const reason = disabledReasons.length ?
102103
html`
103104
<devtools-button class="disabled-reason" .iconName=${'info'} .variant=${Buttons.Button.Variant.ICON} .size=${
104-
Buttons.Button.Size.SMALL} title=${ifDefined(this.#setting.disabledReason())} @click=${
105+
Buttons.Button.Size.SMALL} title=${ifDefined(disabledReasons.join('\n'))} @click=${
105106
onclick}></devtools-button>
106107
` :
107108
LitHtml.nothing;
@@ -111,7 +112,7 @@ export class SettingCheckbox extends HTMLElement {
111112
<label title=${title}>
112113
<input
113114
type="checkbox"
114-
.checked=${this.#setting.disabledReason() ? false : this.#setting.get()}
115+
.checked=${disabledReasons.length ? false : this.#setting.get()}
115116
?disabled=${this.#setting.disabled()}
116117
@change=${this.#checkboxChanged}
117118
jslog=${VisualLogging.toggle().track({click: true}).context(this.#setting.name)}

0 commit comments

Comments
 (0)