Skip to content

Sync filters between Suite and Visual Checks pages#769

Merged
sonic16x merged 7 commits into
masterfrom
sync-filters
Jun 10, 2026
Merged

Sync filters between Suite and Visual Checks pages#769
sonic16x merged 7 commits into
masterfrom
sync-filters

Conversation

@sonic16x

Copy link
Copy Markdown
Contributor

No description provided.

@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown

✅ Component tests succeed

Report

@github-actions

github-actions Bot commented May 24, 2026

Copy link
Copy Markdown

✅ E2E tests succeed

Report

@sonic16x sonic16x force-pushed the sync-filters branch 2 times, most recently from 978a904 to 9765364 Compare May 25, 2026 02:00
@pkg-pr-new

pkg-pr-new Bot commented May 25, 2026

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/html-reporter@769

commit: cc4265f

@sonic16x sonic16x force-pushed the sync-filters branch 7 times, most recently from 38bf835 to b8aeaa9 Compare May 25, 2026 08:10
@sonic16x sonic16x requested a review from shadowusr May 25, 2026 08:20
@sonic16x sonic16x force-pushed the sync-filters branch 2 times, most recently from 2cd42f0 to 62d37df Compare June 7, 2026 15:36

@shadowusr shadowusr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more thing that I think belongs to this PR is syncing selected tree items between two pages. So when user opens certain test and has some visual check in view, we should attempt to open it in visual checks, similarly, when selecting any visual checks and then going to suites page, we should open the exact suite&visual check that was selected on the visual checks page.

In case there's no visual check associated with suite on visual checks page, we should do nothing, preserving old behavior.

}
localStorageWrapper.setItem('app.suitesPage.viewMode', app.suitesPage.viewMode);
localStorageWrapper.setItem('app.visualChecksPage.viewMode', app.visualChecksPage.viewMode);
localStorageWrapper.setItem('app.viewMode', app.viewMode);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work: after page reload, filter resets to "All" regardless of what was chosen previously

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed both problem, now view saved in localStorage (this bug was in master also) and now selected items also synced between pages

@sonic16x sonic16x force-pushed the sync-filters branch 3 times, most recently from d452752 to 032cccb Compare June 9, 2026 20:16

@shadowusr shadowusr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks great! Two things:

  • Could you also please check one more thing, since you’ve been changing the browser filters state logic: does filtering by browser work on the Visual Checks page at all? It doesn’t work on master, and it looks like it probably never did.

  • I’m also still a bit unsure about this flow:

  1. Select a test without a visual check.
  2. Go to the Visual Checks page, where the first item gets auto-selected.
  3. Go back to the Suites page, and we end up on the test that contains the first visual check in the list.

IMO, this feels a bit strange. I would expect this flow to end on the same test that was originally selected. When switching to Visual Checks, we could either show “nothing selected” or avoid treating this auto-selection of the first item as a real user selection.

But this is not critical — please take a look and do what seems best to you.

@sonic16x sonic16x merged commit 362c197 into master Jun 10, 2026
6 checks passed
@sonic16x sonic16x deleted the sync-filters branch June 10, 2026 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants