You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "aminghadersohi (via GitHub)" <gi...@apache.org> on 2026/08/18 20:57:27 UTC
[PR] fix(listview): stop card clicks creating a duplicate history entry [superset]
aminghadersohi opened a new pull request, #43310:
URL: https://github.com/apache/superset/pull/43310
## Why
Clicking a chart or dashboard card and then pressing the browser Back button does not return you to the page you came from — you stay on the chart, and only a second Back press gets you out.
The cause is a double navigation. `ListViewCard` renders its cover (the thumbnail) as a react-router `<Link to={url}>`, while `ChartCard` and `DashboardCard` additionally wrap the whole card in a clickable element that calls `history.push(url)`. Neither stops propagation, so a single click on the cover is handled twice and pushes **two identical history entries**. Back pops the duplicate, which resolves to the same Explore/Dashboard URL, so the navigation looks broken.
This only shows up where cards are rendered with thumbnails — the homepage chart/dashboard sections. The Chart and Dashboard list pages default to table view (card view is behind `LISTVIEWS_DEFAULT_CARD_VIEW`), which uses a plain link and pushes once, which is why those entry points behave correctly.
I kept the whole card clickable rather than removing the wrapper handler, since clicking the title or body — which are not inside the link — must still navigate.
## What
Added a small `isNavigationHandledByLink` helper in `src/views/CRUD/utils.tsx` that reports whether a click originated inside an anchor. `ChartCard` and `DashboardCard` skip their own `history.push` in that case and let the link navigate, so one click produces exactly one history entry.
## How to test
Regression tests added for both cards asserting that a click on the cover produces exactly one `PUSH`, plus a test that clicking outside the cover still navigates. Both new cover tests fail on `master` (the recorded navigations are `['PUSH …', 'PUSH …']`) and pass with the fix.
```
npx jest src/features/charts src/features/dashboards src/features/home/ChartTable.test.tsx \
src/features/home/DashboardTable.test.tsx src/pages/ChartList src/pages/DashboardList src/views/CRUD
# Test Suites: 15 passed, 15 total
# Tests: 201 passed, 201 total
```
Manually: enable `THUMBNAILS`, go to the homepage, click a chart card thumbnail, then press Back — you land back on the homepage instead of staying on the chart.
## Risk & rollback
Low and contained to the two card components. The only behavior change is that a click on the cover no longer double-navigates; clicks on the title, description, and card background are unchanged, and `bulkSelectEnabled` still short-circuits navigation as before. Straight revert if needed.
## Review guidance
Start with `src/views/CRUD/utils.tsx` for the helper and its rationale, then the two one-line call sites. The riskiest assumption is the `closest('a[href]')` check — it deliberately matches any anchor ancestor, so if a card ever gains a non-navigating anchor, that click would stop navigating the card.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(listview): stop card clicks creating a duplicate history entry [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43310:
URL: https://github.com/apache/superset/pull/43310#issuecomment-5338528634
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(listview): stop card clicks creating a duplicate history entry [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43310:
URL: https://github.com/apache/superset/pull/43310#issuecomment-5341987442
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(listview): stop card clicks creating a duplicate history entry [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #43310:
URL: https://github.com/apache/superset/pull/43310#discussion_r3810703518
##########
superset-frontend/src/features/dashboards/DashboardCard.test.tsx:
##########
@@ -101,6 +102,43 @@ test('Renders the modified date', () => {
expect(modifiedDateElement).toBeInTheDocument();
});
+test('clicking the thumbnail navigates to the dashboard exactly once', () => {
+ // The cover is a router link and the whole card is clickable, so a click on
+ // the cover used to be handled twice and pushed two identical entries, which
+ // left the Back button popping the duplicate rather than returning the user
+ // to the page they came from.
+ jest.spyOn(global, 'fetch').mockResolvedValue({
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Unrestored spy in test leaks across tests</b></div>
<div id="fix">
The `jest.spyOn(global, 'fetch')` at line 110 is never restored. Tests that run after this one will still have the mocked fetch, causing cross-test pollution.
</div>
</div>
<small><i>Code Review Run #f222ec</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(listview): stop card clicks creating a duplicate history entry [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43310:
URL: https://github.com/apache/superset/pull/43310#issuecomment-5333971771
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(listview): stop card clicks creating a duplicate history entry [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi merged PR #43310:
URL: https://github.com/apache/superset/pull/43310
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(listview): stop card clicks creating a duplicate history entry [superset]
Posted by "aminghadersohi (via GitHub)" <gi...@apache.org>.
aminghadersohi commented on code in PR #43310:
URL: https://github.com/apache/superset/pull/43310#discussion_r3815890852
##########
superset-frontend/src/features/dashboards/DashboardCard.test.tsx:
##########
@@ -101,6 +102,43 @@ test('Renders the modified date', () => {
expect(modifiedDateElement).toBeInTheDocument();
});
+test('clicking the thumbnail navigates to the dashboard exactly once', () => {
+ // The cover is a router link and the whole card is clickable, so a click on
+ // the cover used to be handled twice and pushed two identical entries, which
+ // left the Back button popping the duplicate rather than returning the user
+ // to the page they came from.
+ jest.spyOn(global, 'fetch').mockResolvedValue({
Review Comment:
Fixed in 8f042cc — added a top-level `afterEach(() => jest.restoreAllMocks())` rather than a trailing `mockRestore()`, since a trailing call would be skipped if an assertion throws.
Also addressed the two additional suggestions in the same commit: the `.gradient-container` selector is now the semantic `getByRole('link')`, and `isNavigationHandledByLink` has direct coverage in `views/CRUD/utils.test.tsx` (link and nested children, non-link targets, an anchor without `href`, and non-element targets).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(listview): stop card clicks creating a duplicate history entry [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43310:
URL: https://github.com/apache/superset/pull/43310#issuecomment-5334110190
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43310?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 66.67%. Comparing base ([`c0ab5f3`](https://app.codecov.io/gh/apache/superset/commit/c0ab5f3385cf0bb3695bc36efd486c0bbe97c82d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2cb4f09`](https://app.codecov.io/gh/apache/superset/commit/2cb4f09de55131a1901223364393bc0465c72654?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #43310 +/- ##
==========================================
- Coverage 66.67% 66.67% -0.01%
==========================================
Files 2876 2876
Lines 164007 164012 +5
Branches 37834 37837 +3
==========================================
+ Hits 109347 109350 +3
- Misses 52514 52516 +2
Partials 2146 2146
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/43310/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/43310/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `73.90% <100.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43310?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(listview): stop card clicks creating a duplicate history entry [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on code in PR #43310:
URL: https://github.com/apache/superset/pull/43310#discussion_r3815892111
##########
superset-frontend/src/features/dashboards/DashboardCard.test.tsx:
##########
@@ -101,6 +102,43 @@ test('Renders the modified date', () => {
expect(modifiedDateElement).toBeInTheDocument();
});
+test('clicking the thumbnail navigates to the dashboard exactly once', () => {
+ // The cover is a router link and the whole card is clickable, so a click on
+ // the cover used to be handled twice and pushed two identical entries, which
+ // left the Back button popping the duplicate rather than returning the user
+ // to the page they came from.
+ jest.spyOn(global, 'fetch').mockResolvedValue({
Review Comment:
<!-- Bito Reply -->
The suggestion provided by the reviewer was appropriate and necessary to prevent cross-test pollution. By adding `jest.restoreAllMocks()` in an `afterEach` block, you have correctly ensured that the `fetch` spy is cleaned up after every test, regardless of whether an assertion throws. This is a standard best practice for managing test mocks in Jest.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org