Skip to content

Commit

Permalink
[ve] Fix logging of the clear filter button.
Browse files Browse the repository at this point in the history
The clear button is actually an uncle of the text field, but we'd like to log it as a child.
This doesn't work with parent provider out of the box, as we scan DOM tree in BFS order, and the text field is not yet processed when we handle clear button. Adjust the logic in getOrCreateState to handle changing parents in case of parent provider.

Bug: 348173254
Change-Id: I9601da151eff059804b24c68c780518f64cebea9
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5677284
Reviewed-by: Simon Zünd <szuend@chromium.org>
Auto-Submit: Danil Somsikov <dsv@chromium.org>
Commit-Queue: Danil Somsikov <dsv@chromium.org>
  • Loading branch information
danilsomsikov authored and Devtools-frontend LUCI CQ committed Jul 4, 2024
1 parent b715053 commit 261767a
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 8 deletions.
2 changes: 2 additions & 0 deletions front_end/ui/legacy/Toolbar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -783,6 +783,8 @@ export class ToolbarInput extends ToolbarItem<ToolbarInput.EventTypes> {
const clearButtonText = i18nString(UIStrings.clearInput);
const clearButton = new Buttons.Button.Button();
clearButton.className = 'toolbar-input-clear-button';
clearButton.setAttribute('jslog', `${VisualLogging.action('clear').track({click: true}).parent('mapped')}`);
VisualLogging.setMappedParent(clearButton, internalPromptElement);
clearButton.variant = Buttons.Button.Variant.ICON;
clearButton.size = Buttons.Button.Size.SMALL;
clearButton.iconName = 'cross-circle-filled';
Expand Down
14 changes: 7 additions & 7 deletions front_end/ui/visual_logging/LoggingState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,19 @@ function nextVeId(): number {
}

export function getOrCreateLoggingState(loggable: Loggable, config: LoggingConfig, parent?: Loggable): LoggingState {
if (state.has(loggable)) {
const currentState = state.get(loggable) as LoggingState;
if (parent && !config.parent && currentState.parent !== getLoggingState(parent)) {
currentState.parent = getLoggingState(parent);
}
return currentState;
}
if (config.parent && parentProviders.has(config.parent) && loggable instanceof Element) {
parent = parentProviders.get(config.parent)?.(loggable);
while (parent instanceof Element && !needsLogging(parent)) {
parent = parent.parentElementOrShadowHost() ?? undefined;
}
}
if (state.has(loggable)) {
const currentState = state.get(loggable) as LoggingState;
if (parent && currentState.parent !== getLoggingState(parent)) {
currentState.parent = getLoggingState(parent);
}
return currentState;
}

const loggableState = {
impressionLogged: false,
Expand Down
7 changes: 6 additions & 1 deletion test/e2e/helpers/application-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,16 @@ export async function filterStorageItems(filter: string) {
await expectVeEvents(
[veImpressionsUnder('Panel: resources > Pane: cookies-data > Toolbar', [veImpression('TextField')])]);
await element.type(filter);
await expectVeEvents([veChange('Panel: resources > Pane: cookies-data > Toolbar > TextField')]);
await expectVeEvents([
veChange('Panel: resources > Pane: cookies-data > Toolbar > TextField'),
veImpressionsUnder(
'Panel: resources > Pane: cookies-data > Toolbar > TextField', [veImpression('Action', 'clear')]),
]);
}

export async function clearStorageItemsFilter() {
await click('.toolbar-input .toolbar-input-clear-button');
await expectVeEvents([veClick('Panel: resources > Pane: cookies-data > Toolbar > TextField > Action: clear')]);
}

export async function clearStorageItems() {
Expand Down

0 comments on commit 261767a

Please sign in to comment.