Skip to content

Fix accept visible images#779

Merged
sonic16x merged 3 commits into
masterfrom
fix-accept-visible
Jul 14, 2026
Merged

Fix accept visible images#779
sonic16x merged 3 commits into
masterfrom
fix-accept-visible

Conversation

@sonic16x

@sonic16x sonic16x commented Jul 9, 2026

Copy link
Copy Markdown
Contributor
Screenshot 2026-07-10 at 02 16 46

Now, if image not expanded (check 4 on screenshot) it will not be accepted

@pkg-pr-new

pkg-pr-new Bot commented Jul 9, 2026

Copy link
Copy Markdown

Open in StackBlitz

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

commit: 3ff0b98

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

✅ Component tests succeed

Report

@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown

✅ E2E tests succeed

Report

Comment thread lib/static/modules/selectors/tree.js Outdated

return visibleResultIds.flatMap(resultId => {
return visibleResultIds.flatMap((resultId) => {
const suitePath = resultsById[resultId].suitePath.join('/');

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 .join("/") looks "magic"

expandedTreeNodesById, looking at its name, wants Id as key

We need suiteId

SuiteId can be derived from result like this:

  const result = state.tree.results.byId[resultId];
  const browser = state.tree.browsers.byId[result.parentId];
  const suiteId = browser.parentId;
  // const suite = state.tree.suites.byId[suiteId];

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.

Screenshot 2026-07-14 at 12 35 56

No, in expandedTreeNodesById we have object like this (see above), it is entityId from useList hook (from gravity-ui)

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.

A little helper to get "treeNodeId" from result would be nice, so we would call "getTreeNodeId" everywhere we need, without having to remember "its constructed as suitePath.join('/')"

Comment thread lib/static/modules/selectors/tree.js Outdated

export const getVisibleImages = createSelector(
getVisibleBrowserIds, getBrowsers, getResults, getImages, getIsStaticImageAccepterEnabled, getIsGui,
(state) => state.ui.suitesPage.expandedTreeNodesById,

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.

Sinse other arguments are named functions, i think we should pull this (state) => state.ui.suitesPage.expandedTreeNodesById into named function (like getExpandedTreeNodesById)

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

@sonic16x
sonic16x force-pushed the fix-accept-visible branch from cb3d589 to 699dc69 Compare July 14, 2026 06:04
@sonic16x
sonic16x force-pushed the fix-accept-visible branch from dc5e1de to 3ff0b98 Compare July 14, 2026 15:03
@sonic16x
sonic16x merged commit 3d3f8c6 into master Jul 14, 2026
6 checks passed
@sonic16x
sonic16x deleted the fix-accept-visible branch July 14, 2026 15:15
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