From 2b36c8eb3e1c3d18b146136ff0da98cc1540375a Mon Sep 17 00:00:00 2001 From: Patrick Brosset Date: Wed, 12 Aug 2020 07:17:55 -0700 Subject: [PATCH] Remove the previewed class when pressing Escape in the .cls pane When typing new classes in the .cls pane, they get applied to the selected element as you type, in order to be previewed live. However, if you press Escape, the classes go away from the .cls pane but they stay on the element itself. They should not. This change fixes that by making sure we re-apply the active classes when this happens. It also adds a new e2e test for the .cls panel. Gif of the fix: https://imgur.com/BjDiEQY.gif Bug: 1114726 Change-Id: I393243b469b416b7fd001a096cda08aa10e8bce2 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2346775 Commit-Queue: Patrick Brosset Reviewed-by: Jack Franklin Reviewed-by: Tony Ross --- front_end/elements/ClassesPaneWidget.js | 1 + test/e2e/elements/BUILD.gn | 1 + test/e2e/elements/classes-pane_test.ts | 45 ++++++++++++++++++++++ test/e2e/helpers/elements-helpers.ts | 50 +++++++++++++++++++++++++ 4 files changed, 97 insertions(+) create mode 100644 test/e2e/elements/classes-pane_test.ts diff --git a/front_end/elements/ClassesPaneWidget.js b/front_end/elements/ClassesPaneWidget.js index 8ba7780476..8b613fd173 100644 --- a/front_end/elements/ClassesPaneWidget.js +++ b/front_end/elements/ClassesPaneWidget.js @@ -87,6 +87,7 @@ export class ClassesPaneWidget extends UI.Widget.Widget { const classNames = this._splitTextIntoClasses(text); if (!classNames.length) { + this._installNodeClasses(node); return; } diff --git a/test/e2e/elements/BUILD.gn b/test/e2e/elements/BUILD.gn index f0e66b39be..9a727742c9 100644 --- a/test/e2e/elements/BUILD.gn +++ b/test/e2e/elements/BUILD.gn @@ -7,6 +7,7 @@ import("../../../third_party/typescript/typescript.gni") node_ts_library("elements") { sources = [ "adornment_test.ts", + "classes-pane_test.ts", "computed-pane-properties_test.ts", "element-breadcrumbs_test.ts", "pseudo-states_test.ts", diff --git a/test/e2e/elements/classes-pane_test.ts b/test/e2e/elements/classes-pane_test.ts new file mode 100644 index 0000000000..c7a17f6ecc --- /dev/null +++ b/test/e2e/elements/classes-pane_test.ts @@ -0,0 +1,45 @@ +// Copyright 2020 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import {beforeEach, describe, it} from 'mocha'; + +import {goToResource} from '../../shared/helper.js'; +import {assertSelectedNodeClasses, toggleClassesPane, toggleClassesPaneCheckbox, typeInClassesPaneInput} from '../helpers/elements-helpers.js'; + +describe('The Classes pane', async () => { + beforeEach(async function() { + await goToResource('elements/simple-styled-page.html'); + await toggleClassesPane(); + }); + + it('can add a class to the element', async () => { + await typeInClassesPaneInput('foo'); + await assertSelectedNodeClasses(['foo']); + }); + + it('can add multiple classes at once', async () => { + await typeInClassesPaneInput('foo bar baz'); + await assertSelectedNodeClasses(['foo', 'bar', 'baz']); + }); + + it('can toggle classes', async () => { + await typeInClassesPaneInput('on off'); + await assertSelectedNodeClasses(['on', 'off']); + + await toggleClassesPaneCheckbox('off'); + await assertSelectedNodeClasses(['on']); + + await toggleClassesPaneCheckbox('off'); + await toggleClassesPaneCheckbox('on'); + await assertSelectedNodeClasses(['off']); + }); + + it('removes the previewed classes on ESC', async () => { + await typeInClassesPaneInput('foo'); + await typeInClassesPaneInput('bar', 'Escape', false); + await typeInClassesPaneInput('baz'); + + await assertSelectedNodeClasses(['foo', 'baz']); + }); +}); diff --git a/test/e2e/helpers/elements-helpers.ts b/test/e2e/helpers/elements-helpers.ts index 3050fbeb5b..e51c7d37bd 100644 --- a/test/e2e/helpers/elements-helpers.ts +++ b/test/e2e/helpers/elements-helpers.ts @@ -16,6 +16,9 @@ const COMPUTED_STYLES_PANEL_SELECTOR = '[aria-label="Computed panel"]'; const COMPUTED_STYLES_SHOW_ALL_SELECTOR = '[aria-label="Show all"]'; const ELEMENTS_PANEL_SELECTOR = '.panel[aria-label="elements"]'; const SECTION_SUBTITLE_SELECTOR = '.styles-section-subtitle'; +const CLS_PANE_SELECTOR = '.styles-sidebar-toolbar-pane'; +const CLS_BUTTON_SELECTOR = '[aria-label="Element Classes"]'; +const CLS_INPUT_SELECTOR = '[aria-placeholder="Add new class"]'; export const assertContentOfSelectedElementsNode = async (expectedTextContent: string) => { const selectedNode = await waitFor(SELECTED_TREE_ELEMENT_SELECTOR); @@ -276,3 +279,50 @@ export const clickOnFirstLinkInStylesPanel = async () => { const stylesPane = await waitFor('div.styles-pane'); await click('div.styles-section-subtitle span.devtools-link', {root: stylesPane}); }; + +export const toggleClassesPane = async () => { + await click(CLS_BUTTON_SELECTOR); +}; + +export const typeInClassesPaneInput = + async (text: string, commitWith: string = 'Enter', waitForNodeChange: Boolean = true) => { + const clsInput = await waitFor(CLS_INPUT_SELECTOR); + await clsInput.type(text); + + if (commitWith) { + const {frontend} = getBrowserAndPages(); + await frontend.keyboard.press(commitWith); + } + + if (waitForNodeChange) { + // Make sure the classes provided in text can be found in the selected element's content. This is important as the + // cls pane applies classes as you type, so we need to wait until all of them are applied. + await waitForFunction(async () => { + const nodeContent = await getContentOfSelectedNode(); + return text.split(' ').every(cls => nodeContent.includes(cls)); + }); + } +}; + +export const toggleClassesPaneCheckbox = async (checkboxLabel: string) => { + const initialValue = await getContentOfSelectedNode(); + + const classesPane = await waitFor(CLS_PANE_SELECTOR); + await click(`input[aria-label="${checkboxLabel}"]`, {root: classesPane}); + + await waitForSelectedNodeChange(initialValue); +}; + +export const assertSelectedNodeClasses = async (expectedClasses: string[]) => { + const nodeText = await getContentOfSelectedNode(); + const match = nodeText.match(/class=\u200B"([^"]*)/); + const classText = match ? match[1] : ''; + const classes = classText.split(/[\s]/).map(className => className.trim()).filter(className => className.length); + + assert.strictEqual( + classes.length, expectedClasses.length, 'Did not find the expected number of classes on the element'); + + for (const expectedClass of expectedClasses) { + assert.include(classes, expectedClass, `Could not find class ${expectedClass} on the element`); + } +};