Skip to content

Commit

Permalink
Revert "Settings: Remove SettingsLandingPageRedesign feature flag, pa…
Browse files Browse the repository at this point in the history
…rt 2."

This reverts commit 6bb3823.

Reason for revert: crbug.com/1277303
tast crostini.NoSharedFolder failing due to errors in crostini
settings page format.

Original change's description:
> Settings: Remove SettingsLandingPageRedesign feature flag, part 2.
>
> Removing all the code that was supporting the case when the flag was
> turned off.
>
>  - Remove overscroll logic in settings_main.html/ts
>  - Remove old state transition logic in main_page_mixin.ts
>  - Move |inSearchMode| Polymer property from main_page_mixin.ts to
>    basic_page.ts as it is now only used there.
>  - Remove advancedToggle UI in basic_page.html/ts
>  - Remove stand-alone safetyCheck entry in settings_menu.html
>
> Fixed: 1271661
> Change-Id: I0c59574d83ae38afa93fff5376d4a206c092af77
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3311377
> Commit-Queue: Demetrios Papadopoulos <dpapad@chromium.org>
> Reviewed-by: John Lee <johntlee@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#948664}

Change-Id: Iea857703ad947664c47addeaa722a4b940e8f8d7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3319597
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Owners-Override: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/main@{#948789}
  • Loading branch information
Joel Hockey authored and Chromium LUCI CQ committed Dec 7, 2021
1 parent e625036 commit 02ed565
Show file tree
Hide file tree
Showing 15 changed files with 434 additions and 38 deletions.
58 changes: 57 additions & 1 deletion chrome/browser/resources/settings/basic_page/basic_page.html
Expand Up @@ -5,14 +5,57 @@
overflow: hidden;
}

:host([advanced-toggling-in-progress_]) {
-webkit-tap-highlight-color: transparent;
}

#advancedToggle {
--ink-color: currentColor;
align-items: center;
background: transparent;
border: none;
box-shadow: none;
color: currentColor;
display: flex;
font-weight: 400;
margin-bottom: 3px;
margin-top: 12px; /* Part of a 48px spacer (33px + 12px + 3px). */
min-height: 32px;
padding: 0 12px;
}

:host-context(.focus-outline-visible) #advancedToggle:focus {
outline: auto 5px -webkit-focus-ring-color;
}

#osSettingsBanner {
background-color: var(--cr-card-background-color);
border-radius: var(--cr-card-border-radius);
box-shadow: var(--cr-card-shadow);
margin-top: 21px;
}

:host(:not([in-search-mode])) settings-section:not([active]) {
#toggleContainer {
align-items: center;
color: var(--cr-primary-text-color);
display: flex;
font: inherit;
justify-content: center;
margin-bottom: 0;
margin-top: 0;
padding-bottom: 0;
padding-top: 0;
}

#toggleSpacer {
padding-top: 33px; /* Part of a 48px spacer (33px + 12px + 3px). */
}

iron-icon {
margin-inline-start: 16px;
}

:host-context([enable-landing-page-redesign]):host(:not([in-search-mode])) settings-section:not([active]) {
display: none;
}
</style>
Expand Down Expand Up @@ -110,6 +153,19 @@
</template>

<template is="dom-if" if="[[showAdvancedSettings_(pageVisibility.advancedSettings)]]">
<template is="dom-if" if="[[showAdvancedToggle_(
inSearchMode, hasExpandedSection_)]]">
<div id="toggleSpacer"></div>
<h2 id="toggleContainer">
<cr-button id="advancedToggle" on-click="advancedToggleClicked_"
aria-expanded$="[[boolToString_(advancedToggleExpanded)]]">
<span>$i18n{advancedPageTitle}</span>
<iron-icon icon="[[getArrowIcon_(advancedToggleExpanded)]]">
</iron-icon>
</cr-button>
</h2>
</template>

<settings-idle-load id="advancedPageTemplate">
<template>
<div id="advancedPage" hidden$="[[!showAdvancedPage_(
Expand Down
59 changes: 47 additions & 12 deletions chrome/browser/resources/settings/basic_page/basic_page.ts
Expand Up @@ -6,6 +6,7 @@
* @fileoverview
* 'settings-basic-page' is the settings page containing the actual settings.
*/
import 'chrome://resources/cr_elements/cr_button/cr_button.m.js';
import 'chrome://resources/cr_elements/hidden_style_css.m.js';
import 'chrome://resources/cr_elements/shared_style_css.m.js';
import 'chrome://resources/cr_elements/shared_vars_css.m.js';
Expand All @@ -32,7 +33,7 @@ import '../default_browser_page/default_browser_page.js';
// </if>

import {assert} from 'chrome://resources/js/assert.m.js';
import {beforeNextRender, html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
import {beforeNextRender, html, microTask, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {SettingsIdleLoadElement} from '../controls/settings_idle_load.js';
import {loadTimeData} from '../i18n_setup.js';
Expand Down Expand Up @@ -111,16 +112,6 @@ export class SettingsBasicPageElement extends SettingsBasicPageElementBase {
},
},

/**
* Whether a search operation is in progress or previous search
* results are being displayed.
*/
inSearchMode: {
type: Boolean,
value: false,
reflectToAttribute: true,
},

advancedToggleExpanded: {
type: Boolean,
value: false,
Expand Down Expand Up @@ -170,7 +161,6 @@ export class SettingsBasicPageElement extends SettingsBasicPageElementBase {
}

pageVisibility: PageVisibility;
inSearchMode: boolean;
advancedToggleExpanded: boolean;
private hasExpandedSection_: boolean;
private showResetProfileBanner_: boolean;
Expand Down Expand Up @@ -358,6 +348,43 @@ export class SettingsBasicPageElement extends SettingsBasicPageElementBase {
new CustomEvent(eventName, {bubbles: true, composed: true, detail}));
}

private advancedToggleClicked_() {
if (this.advancedTogglingInProgress_) {
return;
}

this.advancedTogglingInProgress_ = true;
const toggle =
this.shadowRoot!.querySelector('#toggleContainer') as HTMLElement;
if (!this.advancedToggleExpanded) {
this.advancedToggleExpanded = true;
microTask.run(() => {
this.getIdleLoad_().then(() => {
this.fire_('scroll-to-top', {
top: toggle.offsetTop,
callback: () => {
this.advancedTogglingInProgress_ = false;
}
});
});
});
} else {
this.fire_('scroll-to-bottom', {
bottom: toggle.offsetTop + toggle.offsetHeight + 24,
callback: () => {
this.advancedToggleExpanded = false;
this.advancedTogglingInProgress_ = false;
}
});
}
}

private showAdvancedToggle_(
inSearchMode: boolean, hasExpandedSection: boolean): boolean {
return !inSearchMode && !hasExpandedSection &&
!loadTimeData.getBoolean('enableLandingPageRedesign');
}

/**
* @return Whether to show the basic page, taking into account both routing
* and search state.
Expand All @@ -383,6 +410,14 @@ export class SettingsBasicPageElement extends SettingsBasicPageElementBase {
private showAdvancedSettings_(visibility?: boolean): boolean {
return visibility !== false;
}

private getArrowIcon_(opened: boolean): string {
return opened ? 'cr:arrow-drop-up' : 'cr:arrow-drop-down';
}

private boolToString_(bool: boolean): string {
return bool.toString();
}
}

declare global {
Expand Down
8 changes: 6 additions & 2 deletions chrome/browser/resources/settings/route.ts
Expand Up @@ -16,8 +16,6 @@ function addPrivacyChildRoutes(r: SettingsRoutes) {
r.CLEAR_BROWSER_DATA = r.PRIVACY.createChild('/clearBrowserData');
r.CLEAR_BROWSER_DATA.isNavigableDialog = true;

r.SAFETY_CHECK = r.PRIVACY.createSection('/safetyCheck', 'safetyCheck');

if (loadTimeData.getBoolean('privacyReviewEnabled')) {
r.PRIVACY_REVIEW = r.PRIVACY.createChild('review');
}
Expand Down Expand Up @@ -142,6 +140,12 @@ function createBrowserSettingsRoutes(): SettingsRoutes {
if (visibility.privacy !== false) {
r.PRIVACY = r.BASIC.createSection('/privacy', 'privacy');
addPrivacyChildRoutes(r);

if (loadTimeData.getBoolean('enableLandingPageRedesign')) {
r.SAFETY_CHECK = r.PRIVACY.createSection('/safetyCheck', 'safetyCheck');
} else {
r.SAFETY_CHECK = r.BASIC.createSection('/safetyCheck', 'safetyCheck');
}
}

// <if expr="not chromeos and not lacros">
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/resources/settings/settings.html
@@ -1,5 +1,6 @@
<!doctype html>
<html dir="$i18n{textdirection}" lang="$i18n{language}" class="loading"
$i18n{enableLandingPageRedesignAttribute}
$i18n{enableBrandingUpdateAttribute}>
<head>
<meta charset="utf-8">
Expand Down
10 changes: 10 additions & 0 deletions chrome/browser/resources/settings/settings_main/settings_main.html
@@ -1,4 +1,12 @@
<style include="cr-shared-style cr-hidden-style settings-shared">
#overscroll {
margin-top: 64px;
}

.showing-subpage ~ #overscroll {
display: none;
}

#noSearchResults {
margin-top: 80px;
text-align: center;
Expand Down Expand Up @@ -38,6 +46,7 @@
class="cr-centered-card-container"
prefs="{{prefs}}"
page-visibility="[[pageVisibility]]"
on-showing-section="onShowingSection_"
on-subpage-expand="onShowingSubpage_"
on-showing-main-page="onShowingMainPage_"
in-search-mode="[[inSearchMode_]]"
Expand All @@ -50,3 +59,4 @@
prefs="{{prefs}}">
</settings-about-page>
</template>
<div id="overscroll" style="padding-bottom: [[overscroll_]]px"></div>
71 changes: 71 additions & 0 deletions chrome/browser/resources/settings/settings_main/settings_main.ts
Expand Up @@ -35,6 +35,12 @@ type MainPageVisibility = {
settings: boolean,
};

export interface SettingsMainElement {
$: {
overscroll: HTMLElement,
};
}

const SettingsMainElementBase = RouteObserverMixin(PolymerElement) as
{new (): PolymerElement & RouteObserverMixinInterface};

Expand Down Expand Up @@ -62,6 +68,11 @@ export class SettingsMainElement extends SettingsMainElementBase {
notify: true,
},

overscroll_: {
type: Number,
observer: 'overscrollChanged_',
},

/**
* Controls which main pages are displayed via dom-ifs, based on the
* current route.
Expand Down Expand Up @@ -103,12 +114,51 @@ export class SettingsMainElement extends SettingsMainElementBase {
}

advancedToggleExpanded: boolean;
private overscroll_: number;
private showPages_: MainPageVisibility;
private inSearchMode_: boolean;
private showNoResultsFound_: boolean;
private showingSubpage_: boolean;
toolbarSpinnerActive: boolean;
pageVisibility: PageVisibility;
private boundScroll_: (() => void)|null = null;

private overscrollChanged_() {
assert(!loadTimeData.getBoolean('enableLandingPageRedesign'));
if (!this.overscroll_ && this.boundScroll_) {
this.offsetParent!.removeEventListener('scroll', this.boundScroll_);
window.removeEventListener('resize', this.boundScroll_);
this.boundScroll_ = null;
} else if (this.overscroll_ && !this.boundScroll_) {
this.boundScroll_ = () => {
if (!this.showingSubpage_) {
this.setOverscroll_(0);
}
};
this.offsetParent!.addEventListener('scroll', this.boundScroll_);
window.addEventListener('resize', this.boundScroll_);
}
}

/**
* Sets the overscroll padding. Never forces a scroll, i.e., always leaves
* any currently visible overflow as-is.
* @param opt_minHeight The minimum overscroll height needed.
*/
private setOverscroll_(opt_minHeight?: number) {
const scroller = this.offsetParent;
if (!scroller) {
return;
}
const overscroll = this.$.overscroll;
const visibleBottom = scroller.scrollTop + scroller.clientHeight;
const overscrollBottom = overscroll.offsetTop + overscroll.scrollHeight;
// How much of the overscroll is visible (may be negative).
const visibleOverscroll =
overscroll.scrollHeight - (overscrollBottom - visibleBottom);
this.overscroll_ =
Math.max(opt_minHeight || 0, Math.ceil(visibleOverscroll));
}

/**
* Updates the hidden state of the about and settings pages based on the
Expand All @@ -135,6 +185,27 @@ export class SettingsMainElement extends SettingsMainElementBase {
this.showingSubpage_ = false;
}

/**
* A handler for the 'showing-section' event fired from settings-basic-page,
* indicating that a section should be scrolled into view as a result of a
* navigation.
*/
private onShowingSection_(e: CustomEvent<HTMLElement>) {
assert(!loadTimeData.getBoolean('enableLandingPageRedesign'));

const section = e.detail;
// Calculate the height that the overscroll padding should be set to, so
// that the given section is displayed at the top of the viewport.
// Find the distance from the section's top to the overscroll.
const sectionTop =
(section.offsetParent as HTMLElement).offsetTop + section.offsetTop;
const distance = this.$.overscroll.offsetTop - sectionTop;
const overscroll = Math.max(0, this.offsetParent!.clientHeight - distance);
this.setOverscroll_(overscroll);
section.scrollIntoView();
section.focus();
}

/**
* @return A promise indicating that searching finished.
*/
Expand Down
10 changes: 10 additions & 0 deletions chrome/browser/resources/settings/settings_menu/settings_menu.html
Expand Up @@ -118,6 +118,16 @@
$i18n{autofillPageTitle}
<paper-ripple></paper-ripple>
</a>
<template is="dom-if" if="[[!enableLandingPageRedesign_]]">
<a role="menuitem" href="/safetyCheck"
hidden="[[!pageVisibility.safetyCheck]]"
id="safetyCheck"
class="cr-nav-menu-item">
<iron-icon icon="settings20:safety-check"></iron-icon>
$i18n{safetyCheckSectionTitle}
<paper-ripple></paper-ripple>
</a>
</template>
<a role="menuitem" href="/privacy"
hidden="[[!pageVisibility.privacy]]"
class="cr-nav-menu-item">
Expand Down
Expand Up @@ -23,6 +23,7 @@ import {IronCollapseElement} from 'chrome://resources/polymer/v3_0/iron-collapse
import {IronSelectorElement} from 'chrome://resources/polymer/v3_0/iron-selector/iron-selector.js';
import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';

import {loadTimeData} from '../i18n_setup.js';
import {PageVisibility} from '../page_visibility.js';
import {Route, RouteObserverMixin, RouteObserverMixinInterface, Router} from '../router.js';

Expand Down Expand Up @@ -61,11 +62,18 @@ export class SettingsMenuElement extends SettingsMenuElementBase {
* Dictionary defining page visibility.
*/
pageVisibility: Object,

enableLandingPageRedesign_: {
type: Boolean,
value: () => loadTimeData.getBoolean('enableLandingPageRedesign'),
},

};
}

advancedOpened: boolean;
pageVisibility: PageVisibility;
private enableLandingPageRedesign_: boolean;

currentRouteChanged(newRoute: Route) {
// Focus the initially selected path.
Expand Down

0 comments on commit 02ed565

Please sign in to comment.