Skip to content

GitLab

T
tangled
Commit b44ce49d authored by michaelpg's avatar michaelpg Committed by Commit bot
Browse files
Options

Use one instance of language settings detail menu for all languages 

Wrap the language settings overflow menu in a <cr-shared-menu>. The contents
of the language detail page will be moved into this menu, so it would be
better not to stamp it once for each item in the list of enabled languages.

BUG=630982
TEST=SettingsLanguagesPageBrowserTest
R=stevenjb@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2277633003
Cr-Commit-Position: refs/heads/master@{#414891}
parent 16e132a1
Show whitespace changes
Inline Side-by-side
Showing with 98 additions and 81 deletions
chrome/browser/resources/settings/languages_page/compiled_resources2.gyp
View file @ b44ce49d
......@@ -38,6 +38,7 @@
'dependencies': [
'../compiled_resources2.gyp:route',
'../settings_page/compiled_resources2.gyp:settings_animated_pages',
'<(DEPTH)/ui/webui/resources/cr_elements/cr_shared_menu/compiled_resources2.gyp:cr_shared_menu',
'<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:assert',
'<(DEPTH)/ui/webui/resources/js/compiled_resources2.gyp:cr',
'languages',
......
chrome/browser/resources/settings/languages_page/languages_page.html
View file @ b44ce49d
......@@ -7,6 +7,7 @@
<link rel="import" href="chrome://resources/polymer/v1_0/paper-icon-button/paper-icon-button.html">
<link rel="import" href="chrome://resources/polymer/v1_0/paper-toggle-button/paper-toggle-button.html">
<link rel="import" href="chrome://resources/cr_elements/cr_expand_button/cr_expand_button.html">
<link rel="import" href="chrome://resources/cr_elements/cr_shared_menu/cr_shared_menu.html">
<link rel="import" href="chrome://resources/cr_elements/icons.html">
<link rel="import" href="/icons.html">
<link rel="import" href="/route.html">
......@@ -79,37 +80,8 @@
</div>
</if>
<paper-icon-button id="more-[[item.language.code]]"
icon="cr:more-vert" toggles
active="{{item.optionsMenuOpened}}"
on-tap="stopPropagationHandler_">
icon="cr:more-vert" on-tap="toggleMenu_">
</paper-icon-button>
<iron-dropdown opened="{{item.optionsMenuOpened}}"
horizontal-align="right" vertical-align="auto">
<div class="dropdown-content"
on-tap="stopPropagationHandler_">
<button class="dropdown-item" role="option"
on-tap="onMoveUpTap_"
hidden="[[isFirstLanguage_(index,
languages.enabled.*)]]">
$i18n{moveUp}
</button>
<button class="dropdown-item" role="option"
on-tap="onMoveDownTap_"
hidden="[[isLastLanguage_(index,
languages.enabled.*)]]">
$i18n{moveDown}
</button>
<button class="dropdown-item" role="option"
on-tap="onShowLanguageDetailTap_">
$i18n{languageDetail}
</button>
<button class="dropdown-item" role="option"
on-tap="onRemoveLanguageTap_"
disabled="[[!item.removable]]">
$i18n{removeLanguage}
</button>
</div>
</iron-dropdown>
</div>
</template>
<div class="list-item list-button" on-tap="onAddLanguagesTap_">
......@@ -193,6 +165,31 @@
</div>
</iron-collapse>
</if>
<template is="dom-if" id="menuTemplate">
<cr-shared-menu>
<button class="dropdown-item" role="menuitem"
on-tap="onMoveUpTap_"
hidden="[[isFirstLanguage_(
detailLanguage_, languages.enabled.*)]]">
$i18n{moveUp}
</button>
<button class="dropdown-item" role="menuitem"
on-tap="onMoveDownTap_"
hidden="[[isLastLanguage_(
detailLanguage_, languages.enabled.*)]]">
$i18n{moveDown}
</button>
<button class="dropdown-item" role="menuitem"
on-tap="onShowLanguageDetailTap_">
$i18n{languageDetail}
</button>
<button class="dropdown-item" role="menuitem"
on-tap="onRemoveLanguageTap_"
disabled="[[!detailLanguage_.removable]]">
$i18n{removeLanguage}
</button>
</cr-shared-menu>
</template>
</neon-animatable>
<template is="dom-if" route-path="/languages/edit" no-search>
<settings-subpage page-title="[[detailLanguage_.language.displayName]]">
......
chrome/browser/resources/settings/languages_page/languages_page.js
View file @ b44ce49d
......@@ -67,16 +67,6 @@ Polymer({
this.languageHelper.setUILanguage(tapEvent.model.item.language.code);
},
/**
* Stops tap events on the language options menu, its trigger, or its items
* from bubbling up to the language itself. Tap events on the language are
* handled in onLanguageTap_.
* @param {!Event} e The tap event.
*/
stopPropagationHandler_: function(e) {
e.stopPropagation();
},
/**
* Handler for enabling or disabling spell check.
* @param {!{target: Element, model: !{item: !LanguageState}}} e
......@@ -107,25 +97,23 @@ Polymer({
},
/**
* @param {number} index Index of the language in the list of languages.
* @param {!Object} change Polymer change object for languages.enabled.*.
* @return {boolean} True if the given language is the first one in the list
* of languages.
* @param {!LanguageState} language
* @return {boolean} True if |language| is first in the list of enabled
* languages. Used to hide the "Move up" option.
* @private
*/
isFirstLanguage_: function(index, change) {
return index == 0;
isFirstLanguage_: function(language) {
return language == this.languages.enabled[0];
},
/**
* @param {number} index Index of the language in the list of languages.
* @param {!Object} change Polymer change object for languages.enabled.*.
* @return {boolean} True if the given language is the last one in the list of
* languages.
* @param {!LanguageState} language
* @return {boolean} True if |language| is last in the list of enabled
* languages. Used to hide the "Move down" option.
* @private
*/
isLastLanguage_: function(index, change) {
return index == this.languages.enabled.length - 1;
isLastLanguage_: function(language) {
return language == this.languages.enabled.slice(-1)[0];
},
/**
......@@ -138,38 +126,37 @@ Polymer({
/**
* Moves the language up in the list.
* @param {!{model: !{item: !LanguageState}}} e
* @private
*/
onMoveUpTap_: function(e) {
this.languageHelper.moveLanguage(e.model.item.language.code, -1);
onMoveUpTap_: function() {
this.menu_.closeMenu();
this.languageHelper.moveLanguage(this.detailLanguage_.language.code, -1);
},
/**
* Moves the language down in the list.
* @param {!{model: !{item: !LanguageState}}} e
* @private
*/
onMoveDownTap_: function(e) {
this.languageHelper.moveLanguage(e.model.item.language.code, 1);
onMoveDownTap_: function() {
this.menu_.closeMenu();
this.languageHelper.moveLanguage(this.detailLanguage_.language.code, 1);
},
/**
* Disables the language.
* @param {!{model: !{item: !LanguageState}}} e
* @private
*/
onRemoveLanguageTap_: function(e) {
this.languageHelper.disableLanguage(e.model.item.language.code);
onRemoveLanguageTap_: function() {
this.menu_.closeMenu();
this.languageHelper.disableLanguage(this.detailLanguage_.language.code);
},
/**
* Opens the Language Detail page for the language.
* @param {!{model: !{item: !LanguageState}}} e
* @private
*/
onShowLanguageDetailTap_: function(e) {
this.detailLanguage_ = e.model.item;
onShowLanguageDetailTap_: function() {
this.menu_.closeMenu();
settings.navigateTo(settings.Route.LANGUAGES_DETAIL);
},
......@@ -348,5 +335,26 @@ Polymer({
forceRenderList_: function(tagName) {
this.$$(tagName).$$('iron-list').fire('iron-resize');
},
/**
* Opens or closes the shared menu at the location of the tapped item.
* @param {!Event} e
* @private
*/
toggleMenu_: function(e) {
e.stopPropagation(); // Prevent the tap event from closing the menu.
this.detailLanguage_ =
/** @type {!{model: !{item: !LanguageState}}} */(e).model.item;
// Ensure the template has been stamped.
if (!this.menu_) {
this.$.menuTemplate.if = true;
this.$.menuTemplate.render();
this.menu_ = /** @type {CrSharedMenuElement} */(
this.$$('cr-shared-menu'));
}
this.menu_.toggleMenu(e.target);
},
});
})();
chrome/browser/resources/settings/settings_shared_css.html
View file @ b44ce49d
......@@ -52,7 +52,8 @@
}
iron-dropdown .dropdown-item,
paper-dropdown-menu .dropdown-item {
paper-dropdown-menu .dropdown-item,
cr-shared-menu .dropdown-item {
align-items: center;
background: none;
border: none;
......@@ -80,11 +81,13 @@
}
iron-dropdown .dropdown-content .dropdown-item:not([disabled]),
cr-shared-menu .dropdown-item:not([disabled]),
iron-dropdown .dropdown-content paper-item {
@apply(--settings-actionable);
}
iron-dropdown .dropdown-content .dropdown-item[disabled] {
iron-dropdown .dropdown-content .dropdown-item[disabled],
cr-shared-menu .dropdown-item[disabled] {
opacity: var(--settings-disabled-opacity);
}
......
chrome/test/data/webui/settings/languages_page_browsertest.js
View file @ b44ce49d
......@@ -51,6 +51,7 @@ TEST_F('SettingsLanguagesPageBrowserTest', 'MAYBE_LanguagesPage', function() {
var languagesSection;
var languagesPage;
var languagesCollapse;
var languageHelper;
/**
......@@ -82,10 +83,10 @@ TEST_F('SettingsLanguagesPageBrowserTest', 'MAYBE_LanguagesPage', function() {
advanced.set('pageVisibility.languages', true);
Polymer.dom.flush();
languagesSection = this.getSection(advanced, 'languages');
assertTrue(!!languagesSection);
languagesPage = languagesSection.querySelector('settings-languages-page');
assertTrue(!!languagesPage);
languagesSection = assert(this.getSection(advanced, 'languages'));
languagesPage = assert(
languagesSection.querySelector('settings-languages-page'));
languagesCollapse = languagesPage.$.languagesCollapse;
languageHelper = languagesPage.languageHelper;
return languageHelper.whenReady();
......@@ -108,8 +109,8 @@ TEST_F('SettingsLanguagesPageBrowserTest', 'MAYBE_LanguagesPage', function() {
var actionButton;
setup(function(done) {
var addLanguagesButton = languagesPage.$.languagesCollapse
.querySelector('.list-button:last-of-type');
var addLanguagesButton =
languagesCollapse.querySelector('.list-button:last-of-type');
MockInteractions.tap(addLanguagesButton);
// The page stamps the dialog and registers listeners asynchronously.
......@@ -191,8 +192,7 @@ TEST_F('SettingsLanguagesPageBrowserTest', 'MAYBE_LanguagesPage', function() {
});
});
test('Should not set UI language', function() {
var languagesCollapse = languagesPage.$.languagesCollapse;
test('should not set UI language', function() {
var languageOptionsDropdownTrigger = languagesCollapse.querySelector(
'paper-icon-button');
assertTrue(!!languageOptionsDropdownTrigger);
......@@ -200,7 +200,11 @@ TEST_F('SettingsLanguagesPageBrowserTest', 'MAYBE_LanguagesPage', function() {
// This shouldn't get called.
languageHelper.setUILanguage = assertNotReached;
// Tap the menu trigger twice to open and close the menu.
MockInteractions.tap(languageOptionsDropdownTrigger);
MockInteractions.tap(languageOptionsDropdownTrigger);
languageHelper.setUILanguage = languageHelper.__proto__.setUILanguage;
});
test('remove language', function() {
......@@ -216,7 +220,6 @@ TEST_F('SettingsLanguagesPageBrowserTest', 'MAYBE_LanguagesPage', function() {
Polymer.dom.flush();
// Find the new language item.
var languagesCollapse = languagesPage.$.languagesCollapse;
var items = languagesCollapse.querySelectorAll('.list-item');
var domRepeat = assert(
languagesCollapse.querySelector('template[is="dom-repeat"]'));
......@@ -227,10 +230,14 @@ TEST_F('SettingsLanguagesPageBrowserTest', 'MAYBE_LanguagesPage', function() {
// Open the menu and select Remove.
MockInteractions.tap(item.querySelector('paper-icon-button'));
var removeMenuItem = assert(item.querySelector(
'.dropdown-content .dropdown-item:last-of-type'));
var languageMenu = assert(languagesPage.$$('cr-shared-menu'));
assertTrue(languageMenu.menuOpen);
var removeMenuItem = assert(languageMenu.querySelector(
'.dropdown-item:last-of-type'));
assertFalse(removeMenuItem.disabled);
MockInteractions.tap(removeMenuItem);
assertFalse(languageMenu.menuOpen);
// We should go back down to the original number of enabled languages.
return whenNumEnabledLanguagesBecomes(numEnabled).then(function() {
......@@ -240,10 +247,11 @@ TEST_F('SettingsLanguagesPageBrowserTest', 'MAYBE_LanguagesPage', function() {
});
test('language detail', function() {
var languagesCollapse = languagesPage.$.languagesCollapse;
var languageDetailMenuItem = languagesCollapse.querySelectorAll(
'.dropdown-content .dropdown-item')[2];
assertTrue(!!languageDetailMenuItem);
var languageOptionsDropdownTrigger = languagesCollapse.querySelector(
'paper-icon-button');
MockInteractions.tap(languageOptionsDropdownTrigger);
var languageDetailMenuItem = languagesPage.root.querySelectorAll(
'cr-shared-menu .dropdown-item')[2];
MockInteractions.tap(languageDetailMenuItem);
var languageDetailPage =
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment