rusackas commented on code in PR #37141:
URL: https://github.com/apache/superset/pull/37141#discussion_r3707302154
##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx:
##########
@@ -527,7 +546,15 @@ const DashboardBuilder = () => {
const headerContent = useMemo(
() => (
<>
- {!hideDashboardHeader && <DashboardHeader />}
+ {!hideDashboardHeader && (
+ <DashboardHeader
+ onOpenMobileFilters={
+ !isNotMobile && nativeFiltersEnabled && hasFilters
+ ? () => setMobileFiltersOpen(true)
+ : undefined
+ }
Review Comment:
Yeah, confirmed with `useNativeFilters()` — `hasFilters` only counted
`filterValues`, so a customization-only dashboard mounts a populated drawer but
never gets a trigger to open it. Pushed 0dfb778b: `hasFilters` now also checks
`chartCustomizations.length`, matching what `nativeFiltersEnabled` and the
vertical filter bar already use for the same call.
##########
superset-frontend/src/features/home/RightMenu.tsx:
##########
@@ -644,6 +654,64 @@ const RightMenu = ({
handleLogout,
]);
+ // Build mobile menu items - consumption only (no create/admin actions)
+ const mobileMenuItems = useMemo(() => {
+ const items: MenuItem[] = [];
+
+ // Add Dashboards link at top (from main menu)
+ // Match on the FAB-internal `name`, which is stable across locales
+ // (`label` is translated and would break in non-English deployments)
+ const dashboardsMenu = menu?.find(item => item.name === 'Dashboards');
+ if (dashboardsMenu) {
+ const dashboardUrl = dashboardsMenu.url || '/dashboard/list/';
+ items.push({
+ key: 'dashboards',
+ label: isFrontendRoute(dashboardUrl) ? (
+ <Link to={dashboardUrl}>{t('Dashboards')}</Link>
+ ) : (
+ <Typography.Link href={dashboardUrl}>
+ {t('Dashboards')}
+ </Typography.Link>
+ ),
+ icon: <Icons.DashboardOutlined />,
+ });
+ }
+
+ // Add theme menu (flatten children directly)
+ menuItems.forEach(item => {
+ if (!item || !('key' in item)) return;
+
+ // Only include theme-sub-menu and language picker
+ if (item.key === 'theme-sub-menu' || item.key === 'language-picker') {
+ items.push({ type: 'divider', key: `divider-before-${item.key}` });
Review Comment:
Good catch, confirmed. emits , not . Pushed 0dfb778b fixing the key check
in RightMenu.
##########
superset-frontend/src/features/home/RightMenu.tsx:
##########
@@ -644,6 +654,64 @@ const RightMenu = ({
handleLogout,
]);
+ // Build mobile menu items - consumption only (no create/admin actions)
+ const mobileMenuItems = useMemo(() => {
+ const items: MenuItem[] = [];
+
+ // Add Dashboards link at top (from main menu)
+ // Match on the FAB-internal `name`, which is stable across locales
+ // (`label` is translated and would break in non-English deployments)
+ const dashboardsMenu = menu?.find(item => item.name === 'Dashboards');
+ if (dashboardsMenu) {
+ const dashboardUrl = dashboardsMenu.url || '/dashboard/list/';
+ items.push({
+ key: 'dashboards',
+ label: isFrontendRoute(dashboardUrl) ? (
+ <Link to={dashboardUrl}>{t('Dashboards')}</Link>
+ ) : (
+ <Typography.Link href={dashboardUrl}>
+ {t('Dashboards')}
+ </Typography.Link>
+ ),
+ icon: <Icons.DashboardOutlined />,
+ });
+ }
+
+ // Add theme menu (flatten children directly)
+ menuItems.forEach(item => {
+ if (!item || !('key' in item)) return;
+
+ // Only include theme-sub-menu and language picker
+ if (item.key === 'theme-sub-menu' || item.key === 'language-picker') {
+ items.push({ type: 'divider', key: `divider-before-${item.key}` });
Review Comment:
Good catch, confirmed. `useLanguageMenuItems` emits `language-submenu`, not
`language-picker`. Pushed 0dfb778b fixing the key check in RightMenu.
--
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]
