Skip to content

Commit

Permalink
Settings UI: Add favicon to password view subpage title
Browse files Browse the repository at this point in the history
Mocks: https://screenshot.googleplex.com/Az9wp5Hxe5MyXjt
Implementation: https://screenshot.googleplex.com/AgwJbf3yFAcYCvv

Bug: 1298027
Change-Id: I8a57aa820090bfb615027d51ad89ab591d8a5f4d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3563161
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Commit-Queue: Adem Derinel <derinel@google.com>
Cr-Commit-Position: refs/heads/main@{#988476}
  • Loading branch information
deephand authored and Chromium LUCI CQ committed Apr 4, 2022
1 parent 3bb220a commit 2ef50d3
Show file tree
Hide file tree
Showing 7 changed files with 62 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@
</template>
<template is="dom-if" if="[[enablePasswordViewPage_]]">
<template is="dom-if" route-path="/passwords/view">
<settings-subpage page-title="[[credential.urls.shown]]">
<settings-subpage page-title="[[credential.urls.shown]]"
favicon-site-url="[[credential.urls.link]]">
<password-view credential="{{credential}}">
</password-view>
</settings-subpage>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ export class PasswordViewElement extends PasswordViewElementBase {
this.credential = {
urls: {
shown: site,
link: site,
}
} as MultiStorePasswordUiEntry;
}
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/resources/settings/chromeos/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -372,6 +372,7 @@ preprocess_if_expr("copy_browser_settings_tsc") {
"settings_page/settings_animated_pages.js",
"settings_page/settings_section.js",
"settings_page/settings_subpage.js",
"site_favicon.js",
]

in_files = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,13 @@

#title-icon {
height: 36px;
width: 36px;
}

#title-icon,
#favicon {
margin-inline-end: 12px;
margin-inline-start: 2px;
width: 36px;
}

#closeButton {
Expand Down Expand Up @@ -70,6 +74,10 @@
<template is="dom-if" if="[[titleIcon]]">
<img id="title-icon" src="[[titleIcon]]" aria-hidden="true">
</template>
<template is="dom-if" if="[[faviconSiteUrl]]">
<site-favicon id="favicon" url="[[faviconSiteUrl]]" aria-hidden="true">
</site-favicon>
</template>
<h1 class="cr-title-text">[[pageTitle]]</h1>
<slot name="subpage-title-extra"></slot>
<template is="dom-if" if="[[learnMoreUrl]]">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import '//resources/cr_elements/icons.m.js';
import '//resources/cr_elements/shared_style_css.m.js';
import '//resources/polymer/v3_0/paper-spinner/paper-spinner-lite.js';
import '../settings_shared_css.js';
import '../site_favicon.js';

import {CrSearchFieldElement} from '//resources/cr_elements/cr_search_field/cr_search_field.js';
import {FindShortcutMixin, FindShortcutMixinInterface} from '//resources/cr_elements/find_shortcut_mixin.js';
Expand Down Expand Up @@ -54,8 +55,12 @@ export class SettingsSubpageElement extends SettingsSubpageElementBase {
return {
pageTitle: String,

/** Setting this will display the icon at the given URL. */
titleIcon: String,

/** Setting this will display the favicon of the website. */
faviconSiteUrl: String,

learnMoreUrl: String,

/** Setting a |searchLabel| will enable search. */
Expand Down Expand Up @@ -118,6 +123,7 @@ export class SettingsSubpageElement extends SettingsSubpageElementBase {

pageTitle: string;
titleIcon: string;
faviconSiteUrl: string;
learnMoreUrl: string;
searchLabel: string;
searchTerm: string;
Expand Down
28 changes: 26 additions & 2 deletions chrome/test/data/webui/settings/autofill_page_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,18 @@
// found in the LICENSE file.

// clang-format off
import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
import {DomIf, flush} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {AutofillManagerImpl, PasswordsSectionElement, PaymentsManagerImpl, SettingsAutofillSectionElement, SettingsPaymentsSectionElement} from 'chrome://settings/lazy_load.js';
import {buildRouter, Router, routes} from 'chrome://settings/settings.js';
import {CrSettingsPrefs, MultiStoreExceptionEntry, MultiStorePasswordUiEntry, OpenWindowProxyImpl, PasswordManagerImpl, SettingsAutofillPageElement, SettingsPluralStringProxyImpl, SettingsPrefsElement} from 'chrome://settings/settings.js';
import {assertDeepEquals, assertEquals, assertNotEquals} from 'chrome://webui-test/chai_assert.js';
import {SettingsRoutes} from 'chrome://settings/settings_routes.js';
import {assertDeepEquals, assertEquals, assertNotEquals, assertTrue} from 'chrome://webui-test/chai_assert.js';
import {TestPluralStringProxy} from 'chrome://webui-test/test_plural_string_proxy.js';
import {flushTasks} from 'chrome://webui-test/test_util.js';

import {FakeSettingsPrivate} from './fake_settings_private.js';
import {AutofillManagerExpectations, createAddressEntry, createCreditCardEntry, createExceptionEntry, createPasswordEntry, PaymentsManagerExpectations, TestAutofillManager, TestPaymentsManager} from './passwords_and_autofill_fake_data.js';
import {AutofillManagerExpectations, createAddressEntry, createCreditCardEntry, createExceptionEntry, createMultiStorePasswordEntry, createPasswordEntry, PaymentsManagerExpectations, TestAutofillManager, TestPaymentsManager} from './passwords_and_autofill_fake_data.js';
import {makeCompromisedCredential} from './passwords_and_autofill_fake_data.js';
import {TestOpenWindowProxy} from './test_open_window_proxy.js';
import {PasswordManagerExpectations,TestPasswordManagerProxy} from './test_password_manager_proxy.js';
Expand Down Expand Up @@ -357,4 +361,24 @@ suite('PasswordsUITest', function() {
.querySelector<HTMLElement>(
'#passwordManagerSubLabel')!.innerText.trim());
});

test('Credential urls is used in the subpage header', async function() {
const SHOWN_URL = 'www.google.com';
loadTimeData.overrideValues({enablePasswordNotes: true});
Router.resetInstanceForTesting(buildRouter());
routes.PASSWORD_VIEW =
(Router.getInstance().getRoutes() as SettingsRoutes).PASSWORD_VIEW;
const autofillSection = createAutofillPageSection();
autofillSection.credential =
createMultiStorePasswordEntry({url: SHOWN_URL, deviceId: 1});

Router.getInstance().navigateTo(routes.PASSWORD_VIEW);
await flushTasks();
const subpage =
autofillSection.shadowRoot!.querySelector('settings-subpage');

assertTrue(!!subpage);
assertEquals(`http://${SHOWN_URL}/login`, subpage.faviconSiteUrl);
assertEquals(SHOWN_URL, subpage.pageTitle);
});
});
17 changes: 17 additions & 0 deletions chrome/test/data/webui/settings/settings_subpage_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,23 @@ suite('SettingsSubpage', function() {
assertEquals('ltr', icon!.getAttribute('dir'));
});

test('favicon', function() {
const subpage = document.createElement('settings-subpage');
document.body.appendChild(subpage);
flush();

// No favicon is shown when the URL is not given.
assertFalse(!!subpage.shadowRoot!.querySelector('site-favicon'));

subpage.faviconSiteUrl = 'https://www.chromium.org';
flush();

// Favicon is shown when the URL is specified.
const favicon = subpage.shadowRoot!.querySelector('site-favicon');
assertTrue(!!favicon);
assertEquals(subpage.faviconSiteUrl, favicon.url);
});

test('clear search (event)', function() {
const subpage = document.createElement('settings-subpage');
// Having a searchLabel will create the cr-search-field.
Expand Down

0 comments on commit 2ef50d3

Please sign in to comment.