codeant-ai-for-open-source[bot] commented on code in PR #37065:
URL: https://github.com/apache/superset/pull/37065#discussion_r3485753651
##########
superset-frontend/src/dashboard/components/nativeFilters/state.ts:
##########
@@ -332,12 +333,26 @@ export function useSelectFiltersInScope(filters: (Filter
| Divider)[]) {
if (filterInScope) {
filtersInScope.push(filter);
} else {
- filtersOutOfScope.push(filter);
+ // Hide tab-scoped filters when their tab exists but is not active.
+ // Show in "Out of Scope" if it's a global filter or scoped to
non-existent tabs.
+ const hasExplicitTabScope =
+ filter.type !== NativeFilterType.Divider &&
+ filter.scope?.rootPath &&
+ filter.scope.rootPath.length > 0 &&
+ filter.scope.rootPath.some(
+ id =>
+ id !== DASHBOARD_ROOT_ID &&
+ dashboardLayout[id]?.type === TAB_TYPE,
+ );
+
+ if (!hasExplicitTabScope) {
+ filtersOutOfScope.push(filter);
+ }
Review Comment:
**Suggestion:** The new hide logic only treats `scope.rootPath` tab IDs as
tab-scoped and ignores filters that are scoped by `chartsInScope` (which can
still be tab-bound at runtime). This leaves a real gap where filters out of
scope due inactive tab charts are still added to the out-of-scope section, so
the behavior remains inconsistent. Include a `chartsInScope`-based tab-scope
check (via chart parent tabs) when deciding whether to suppress
`filtersOutOfScope`. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Tab-bound chart-scoped filters shown in out-of-scope panel.
- ⚠️ Inconsistent UX between tab-scoped and chart-scoped filters.
- ⚠️ Users see filters for inactive tabs, causing confusion.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In a tabbed dashboard, define a native filter whose persisted
configuration includes
`chartsInScope` pointing to charts on a non-active tab, but whose
`scope.rootPath` no
longer contains any tab layout IDs (e.g., only `ROOT_ID`). Such a filter
shape is
supported by the data model: `chartsInScope` and `tabsInScope` are persisted
and updated
via `setInScopeStatusOfFilters` in
`superset-frontend/src/dashboard/actions/nativeFilters.ts` lines 7–21, while
the filter
type allows `chartsInScope`/`tabsInScope` independent of `scope.rootPath` as
shown in
`createMockFilter` in
`superset-frontend/src/dashboard/reducers/nativeFilters.test.ts`
lines 7–42.
2. Render a dashboard view where active tabs (from Redux
`dashboardState.activeTabs`) do
not include the tab(s) hosting those charts. The FilterBar’s
`FilterControls` component at
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterControls.tsx`
lines 19–22 computes `filtersWithValues` and calls
`useSelectFiltersInScope(filtersWithValues)`, which is implemented in
`superset-frontend/src/dashboard/components/nativeFilters/state.ts` lines
58–76.
3. Inside `useSelectFiltersInScope`, the `isFilterInScope` hook (defined in
`state.ts`
lines 13–55) uses `filter.chartsInScope` together with
`selectChartTabParents` (lines 1–8)
and the current `activeTabs` to set `filterInScope`. Because all charts are
on non-active
tabs, `isChartInScope` is false and, with `hasChartsInScope` true,
`isFilterInScope`
returns `false` for this filter.
4. Still in `useSelectFiltersInScope` (`state.ts` lines 67–80, 339–378), the
code computes
`hasExplicitTabScope` only from `filter.scope.rootPath` and
`dashboardLayout[id]?.type ===
TAB_TYPE`. Since `scope.rootPath` no longer contains any IDs whose layout
item is a
`TAB_TYPE`, `hasExplicitTabScope` is false even though `chartsInScope` (and
derived
`tabsInScope` via `calculateScopes` in
`superset-frontend/src/dashboard/util/calculateScopes.ts` lines 19–33)
clearly bind the
filter to specific tabs. The filter is therefore pushed into
`filtersOutOfScope` (`if
(!hasExplicitTabScope) { filtersOutOfScope.push(filter); }` at `state.ts`
lines 339–350).
`FilterControls` passes `filtersOutOfScope` to
`FiltersOutOfScopeCollapsible` in
`superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FiltersOutOfScopeCollapsible/index.tsx`
lines 26–52, causing the filter to be rendered under “Filters out of scope”
instead of
being fully hidden like other tab-scoped filters. This demonstrates that
chart-scoped,
tab-bound filters remain visible in the out-of-scope section when their tab
is inactive
because the hide logic ignores `chartsInScope`/`tabsInScope` when determining
`hasExplicitTabScope`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=dbf4a16361c04144b66cff10b5a819c4&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=dbf4a16361c04144b66cff10b5a819c4&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/dashboard/components/nativeFilters/state.ts
**Line:** 338:350
**Comment:**
*Incomplete Implementation: The new hide logic only treats
`scope.rootPath` tab IDs as tab-scoped and ignores filters that are scoped by
`chartsInScope` (which can still be tab-bound at runtime). This leaves a real
gap where filters out of scope due inactive tab charts are still added to the
out-of-scope section, so the behavior remains inconsistent. Include a
`chartsInScope`-based tab-scope check (via chart parent tabs) when deciding
whether to suppress `filtersOutOfScope`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37065&comment_hash=07879d2bbf6c1ddb3582eedc8664c3b401c68b3778dc1670a484cc62a7ddbda5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37065&comment_hash=07879d2bbf6c1ddb3582eedc8664c3b401c68b3778dc1670a484cc62a7ddbda5&reaction=dislike'>👎</a>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
