rusackas commented on code in PR #37141:
URL: https://github.com/apache/superset/pull/37141#discussion_r3717520531
##########
docs/docs/using-superset/mobile-experience.mdx:
##########
@@ -0,0 +1,89 @@
+---
+title: Mobile Experience
+sidebar_position: 7
+version: 1
+---
+
+import useBaseUrl from "@docusaurus/useBaseUrl";
+
+# Mobile Experience
+
+Superset ships an optional, consumption-only mobile experience for viewing
+dashboards on phones and other small screens. When enabled, screens below
+768px wide get a layout built for touch: dashboards render their charts
Review Comment:
Docs say screens below 768px, code uses max-width: 767px (screenSMMax).
Those are the same set for integer pixel widths, so nothing to change here.
##########
superset-frontend/src/dashboard/components/SliceHeader/index.tsx:
##########
@@ -229,7 +230,9 @@ const SliceHeader = forwardRef<HTMLDivElement,
SliceHeaderProps>(
0,
);
- const canExplore = !editMode && supersetCanExplore;
+ // Consumption-only mobile mode: no explore link, no chart controls
+ const isMobile = useIsMobile();
Review Comment:
Added a mobile-viewport test confirming the explore prompt and
SliceHeaderControls are both hidden.
##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx:
##########
@@ -963,3 +979,62 @@ test('withholds the empty-state edit action while
previewing a version', async (
queryByRole('button', { name: 'Edit the dashboard' }),
).not.toBeInTheDocument();
});
+
+// Mobile support tests
+// Note: The main mobile tests require mocking useBreakpoint to return mobile
breakpoints
+// which is done at the module level. These tests verify mobile-related
component behavior.
+
+test('should not render filter bar panel on desktop when nativeFiltersEnabled
is false', () => {
+ (useStoredSidebarWidth as jest.Mock).mockImplementation(() => [
+ 100,
+ jest.fn(),
+ ]);
+ (fetchFaveStar as jest.Mock).mockReturnValue({ type: 'mock-action' });
+ (setActiveTab as jest.Mock).mockReturnValue({ type: 'mock-action' });
+
+ jest.spyOn(useNativeFiltersModule, 'useNativeFilters').mockReturnValue({
+ showDashboard: true,
+ missingInitialFilters: [],
+ dashboardFiltersOpen: true,
+ toggleDashboardFiltersOpen: jest.fn(),
+ nativeFiltersEnabled: false,
+ hasFilters: false,
+ });
+
+ const { queryByTestId } = render(<DashboardBuilder />, {
+ useRedux: true,
+ store: storeWithState({
+ ...mockState,
+ dashboardLayout: undoableDashboardLayout,
+ }),
+ useDnd: true,
+ useTheme: true,
+ useRouter: true,
+ });
+
+ // Filter panel should not be present when native filters are disabled
+ expect(queryByTestId('dashboard-filters-panel')).not.toBeInTheDocument();
+});
Review Comment:
Yeah, this duplicated existing coverage without exercising anything
mobile-specific despite the section comment. Removed it.
--
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]
