Local NTP: only show "Remove suggestion" (X) on hover

* Show X on row hover * Show (X) (background) on icon hover * Reserve space if any 1+ matches are removable Fixed: 1002689 Change-Id: Idd1f4a623e013893d6e376eedaff3ea2ee26fdc6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1842622 Commit-Queue: Dan Beam <[email protected]> Reviewed-by: Demetrios Papadopoulos <[email protected]> Auto-Submit: Dan Beam <[email protected]> Cr-Commit-Position: refs/heads/master@{#704304}
xuefu · Oct 9, 2019 · 1c49d17 · 1c49d17
1 parent 6023131
commit 1c49d17
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 14 deletions.
diff --git a/chrome/browser/resources/local_ntp/local_ntp.css b/chrome/browser/resources/local_ntp/local_ntp.css
@@ -236,7 +236,7 @@ body.hide-fakebox #fakebox {
   background-position-x: calc(100% - 16px);
 }
 
-#realbox-matches a.removable {
+#realbox-matches.removable a {
   padding-inline-end: 48px;
 }
 
@@ -304,15 +304,16 @@ html[dir=rtl] #realbox-matches .clock-icon {
 }
 
 #realbox-matches .remove-icon {
-  -webkit-appearance: none;
+  height: 100%;
+  width: 100%;
+}
+
+#realbox-matches a:-webkit-any(:hover, :focus-within) .remove-icon {
   -webkit-mask-image: url(../../../../ui/webui/resources/images/icon_clear.svg);
   -webkit-mask-position: center;
   -webkit-mask-repeat: no-repeat;
   -webkit-mask-size: 16px;
   background-color: rgb(var(--GG900-rgb));
-  border: none;
-  height: 100%;
-  width: 100%;
 }
 
 #fakebox > input {
@@ -837,13 +838,11 @@ html[dir=rtl] #error-notice.has-link #error-notice-msg {
 }
 
 #promo > div .dismiss-icon {
-  -webkit-appearance: none;
   -webkit-mask-image: url(../../../../ui/webui/resources/images/icon_clear.svg);
   -webkit-mask-position: center center;
   -webkit-mask-repeat: no-repeat;
   -webkit-mask-size: 16px;
   background-color: rgb(var(--dismiss-background-rgb));
-  border: none;
   display: block;
   height: 100%;
   outline: none;

diff --git a/chrome/browser/resources/local_ntp/local_ntp.js b/chrome/browser/resources/local_ntp/local_ntp.js
@@ -1448,7 +1448,7 @@ function populateAutocompleteMatches(matches) {
 
       remove.appendChild(icon);
       matchEl.appendChild(remove);
-      matchEl.classList.add(CLASSES.REMOVABLE);
+      realboxMatchesEl.classList.add(CLASSES.REMOVABLE);
     }
 
     // TODO(crbug.com/1002689): set a more useful aria-label on |matchEl|, as

diff --git a/chrome/test/data/local_ntp/realbox_browsertest.js b/chrome/test/data/local_ntp/realbox_browsertest.js
@@ -19,6 +19,7 @@ test.realbox.IDS = {
  * @const
  */
 test.realbox.CLASSES = {
+  REMOVABLE: 'removable',
   REMOVE_ICON: 'remove-icon',
   SELECTED: 'selected',
   SHOW_MATCHES: 'show-matches',
@@ -513,6 +514,9 @@ test.realbox.testUnsupportedDeletion = function() {
   test.realbox.realboxEl.dispatchEvent(keyEvent);
   assertFalse(keyEvent.defaultPrevented);
 
+  const matchesEl = $(test.realbox.IDS.REALBOX_MATCHES);
+  assertFalse(matchesEl.classList.contains(test.realbox.CLASSES.REMOVABLE));
+
   // The below 2 statements shouldn't really happen in updated code but isn't
   // terrible idea to keep testing for now. This is because SupportsDeletion()
   // is now propagated to the client, so we shouldn't allow users (via the UI)
@@ -533,6 +537,9 @@ test.realbox.testSupportedDeletion = function() {
   chrome.embeddedSearch.searchBox.onqueryautocompletedone(
       {input: test.realbox.realboxEl.value, matches});
 
+  const matchesEl = $(test.realbox.IDS.REALBOX_MATCHES);
+  assertTrue(matchesEl.classList.contains(test.realbox.CLASSES.REMOVABLE));
+
   const downArrow = new KeyboardEvent('keydown', {
     bubbles: true,
     cancelable: true,
@@ -541,9 +548,9 @@ test.realbox.testSupportedDeletion = function() {
   test.realbox.realboxEl.dispatchEvent(downArrow);
   assertTrue(downArrow.defaultPrevented);
 
-  const matchEls = $(test.realbox.IDS.REALBOX_MATCHES).children;
-  assertEquals(2, matchEls.length);
-  assertTrue(matchEls[1].classList.contains(test.realbox.CLASSES.SELECTED));
+  assertEquals(2, matchesEl.children.length);
+  assertTrue(
+      matchesEl.children[1].classList.contains(test.realbox.CLASSES.SELECTED));
 
   const shiftDelete = new KeyboardEvent('keydown', {
     bubbles: true,
@@ -571,9 +578,10 @@ test.realbox.testRemoveIcon = function() {
   chrome.embeddedSearch.searchBox.onqueryautocompletedone(
       {input: test.realbox.realboxEl.value, matches});
 
-  const sel = `#${test.realbox.IDS.REALBOX_MATCHES}
-               .${test.realbox.CLASSES.REMOVE_ICON}`;
-  document.querySelector(sel).click();
+  const matchesEl = $(test.realbox.IDS.REALBOX_MATCHES);
+  assertTrue(matchesEl.classList.contains(test.realbox.CLASSES.REMOVABLE));
+
+  matchesEl.querySelector(`.${test.realbox.CLASSES.REMOVE_ICON}`).click();
 
   assertEquals(1, test.realbox.deletedLines.length);
   assertEquals(0, test.realbox.deletedLines[0]);