Skip to content

Commit

Permalink
[NTP] Done button should always be enabled.
Browse files Browse the repository at this point in the history
Bug: 937570
Change-Id: I12149327740ff490f8c1a3e14afc1d5fd028db5d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1733206
Commit-Queue: Kyle Milka <kmilka@chromium.org>
Reviewed-by: Gayane Petrosyan <gayane@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685824}
  • Loading branch information
Kyle Milka authored and Commit Bot committed Aug 10, 2019
1 parent c037fa4 commit 9c2674b
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 47 deletions.
28 changes: 1 addition & 27 deletions chrome/browser/resources/local_ntp/customize.js
Original file line number Diff line number Diff line change
Expand Up @@ -858,25 +858,6 @@ customize.richerPicker_isShortcutOptionSelected = function() {
return notPreselectedType || notPreselectedHidden;
};

/**
* Return true if any option is selected. Used to enable the 'done' button.
*/
customize.richerPicker_isOptionSelected = function() {
return customize.isBackgroundOptionSelected() ||
customize.isColorOptionSelected() ||
customize.richerPicker_isShortcutOptionSelected();
};

/**
* Enable the 'done' button if any option is selected. If no option is selected,
* disable the 'done' button.
*/
customize.richerPicker_maybeToggleDone = function() {
const enable = customize.richerPicker_isOptionSelected();
$(customize.IDS.MENU_DONE).disabled = !enable;
$(customize.IDS.MENU_DONE).tabIndex = enable ? 0 : -1;
};

/**
* Apply styling to a selected option in the richer picker (i.e. the selected
* background image, shortcut type, and color).
Expand Down Expand Up @@ -1008,7 +989,6 @@ customize.richerPicker_selectBackgroundTile = function(tile) {
collectionId: '',
};
customize.richerPicker_applySelectedState(tile);
customize.richerPicker_maybeToggleDone();

// Don't apply a preview for a preselected image, as it's already the
// page background.
Expand All @@ -1035,7 +1015,6 @@ customize.richerPicker_selectShortcutType = function(shortcutType) {
}
customize.selectedOptions.shortcutType = shortcutType;
customize.richerPicker_applySelectedState(shortcutType);
customize.richerPicker_maybeToggleDone();
};

/**
Expand All @@ -1051,7 +1030,6 @@ customize.richerPicker_toggleShortcutHide = function(areHidden) {
$(customize.IDS.SHORTCUTS_HIDE_TOGGLE).checked = areHidden;

customize.selectedOptions.shortcutsAreHidden = areHidden;
customize.richerPicker_maybeToggleDone();
};

/**
Expand Down Expand Up @@ -1497,7 +1475,7 @@ customize.richerPicker_cancelCustomization = function() {
* picker.
*/
customize.richerPicker_applyCustomization = function() {
if (customize.selectedOptions.backgroundData) {
if (customize.isBackgroundOptionSelected()) {
customize.setBackground(
customize.selectedOptions.backgroundData.url,
customize.selectedOptions.backgroundData.attr1,
Expand Down Expand Up @@ -1903,9 +1881,6 @@ customize.initCustomBackgrounds = function(showErrorNotification) {
const doneInteraction = function(event) {
const done = configData.richerPicker ? $(customize.IDS.MENU_DONE) :
$(customize.IDS.DONE);
if (done.disabled) {
return;
}
if (configData.richerPicker) {
ntpApiHandle.logEvent(customize.LOG_TYPE.NTP_CUSTOMIZATION_MENU_DONE);
customize.richerPicker_applyCustomization();
Expand Down Expand Up @@ -2184,7 +2159,6 @@ customize.updateColorsMenuTileSelection = function(tile) {
}
customize.selectedOptions.color = tile;
customize.richerPicker_applySelectedState(tile);
customize.richerPicker_maybeToggleDone();
};

/**
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/resources/local_ntp/local_ntp.html
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@
class="bg-sel-footer-button paper secondary ripple"
title="$i18n{cancelButton}">$i18n{cancelButton}</button>
<button id="menu-done" class="bg-sel-footer-button paper primary ripple"
title="$i18n{doneButton}" disabled>$i18n{doneButton}</button>
title="$i18n{doneButton}">$i18n{doneButton}</button>
</div>
</dialog>

Expand Down
18 changes: 0 additions & 18 deletions chrome/test/data/local_ntp/customize_menu_browsertest.js
Original file line number Diff line number Diff line change
Expand Up @@ -715,24 +715,6 @@ test.customizeMenu.testColors_PreselectDefault = function() {
.classList.contains(test.customizeMenu.CLASSES.SELECTED));
};

/**
* Test 'Done' button enabled/disabled when preselect is present.
*/
test.customizeMenu.testColors_DoneButtonWithPreselect = function() {
test.customizeMenu.mockThemeBackgroundInfo = {usingDefaultTheme: true};
init();
$(test.customizeMenu.IDS.EDIT_BG).click();
$(test.customizeMenu.IDS.COLORS_BUTTON).click();

assertTrue($(test.customizeMenu.IDS.MENU_DONE).disabled);

!!$('color_0').click();
assertFalse($(test.customizeMenu.IDS.MENU_DONE).disabled);

$(test.customizeMenu.IDS.COLORS_DEFAULT_ICON).click();
assertTrue($(test.customizeMenu.IDS.MENU_DONE).disabled);
};

/**
* Test preselect color tile.
*/
Expand Down
2 changes: 1 addition & 1 deletion chrome/test/data/local_ntp/local_ntp_browsertest.html
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@
class="bg-sel-footer-button paper secondary ripple"
title="$i18n{cancelButton}">$i18n{cancelButton}</button>
<button id="menu-done" class="bg-sel-footer-button paper primary ripple"
title="$i18n{doneButton}" disabled>$i18n{doneButton}</button>
title="$i18n{doneButton}">$i18n{doneButton}</button>
</div>
</dialog>

Expand Down

0 comments on commit 9c2674b

Please sign in to comment.