Open records on a full page instead of a side panel on mobile (#23474)
On mobile the side panel covers the whole screen, so a record opened in
it arrives cramped behind an "Open" button offering the full page it
should have gone to in the first place.
`useResolveOpenRecordIn` already forces `RECORD_PAGE` on mobile via
`canDisplaySidePanel: !isMobile`, but it is a resolver callers have to
opt into, and only five do. Thirteen other call sites reach
`useOpenRecordInSidePanel` directly and get a panel on every device,
including:
- `TaskRow` and `NoteTile`, the activity lists inside a record's tabs
- `EventRowActivity`, `EventCardMessage`, `EventRowGenericLinked` on the
timeline
- `SidePanelSearchRecordsPage`, `EmailThreadPreview`,
`useOpenCreateActivityDrawer`, `useAddNewRecordAndOpenSidePanel`
## Change
Decide it inside `useOpenRecordInSidePanel` rather than at each call
site, so no caller can wedge a record into a panel by forgetting to ask.
On mobile it closes the panel and navigates to `AppPath.RecordShowPage`,
then returns before any of the side-panel setup runs.
Two details carried over so the redirect is not lossy:
- `setRecordPageActiveTabId` still runs first, so a caller passing `tab`
lands on the right tab.
- `isNewRecord` forwards `{ isNewRecord, objectRecordId,
labelIdentifierFieldName }` as navigation state, mirroring what
`useCreateNewIndexRecord` already does on its `RECORD_PAGE` branch, so a
freshly created record still opens its title for naming instead of
arriving untitled.
Side-panel-only effects are skipped rather than lost.
`runWorkflowRunOpeningInSidePanelEffects` ends in
`openWorkflowRunViewStepInSidePanel`, which auto-opens a step *in the
panel*; with no panel there is nothing for it to do, and the workflow
run's record page renders its own diagram.
The two hooks that already branch on `useResolveOpenRecordIn`
(`useOpenRecordFromIndexView`, `useCreateNewIndexRecord`) never call
into this path on mobile, so this is a no-op for them rather than a
double navigation.
Uses `useIsMobile` rather than `useIsTouchDevice`, matching
`useResolveOpenRecordIn`: this is a question of whether there is room
for a panel, not of how the user points.
## Testing
At 390x844, opening the search side panel and tapping a result now
navigates to `/object/person/<id>` with the panel closed, where it
previously stayed in the panel. Typecheck and lint clean.
---
_Generated by [Claude
Code](https://claude.ai/code/session_018gcsCQbuTMsyFWv874p25Q)_
<!-- This is an auto-generated description by cubic. -->
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/23474?utm_source=github"
target="_blank" rel="noopener noreferrer"
data-no-image-dialog="true"><picture><source
media="(prefers-color-scheme: dark)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source
media="(prefers-color-scheme: light)"
srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img
alt="Review in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a>
<!-- End of auto-generated description by cubic. -->
This commit is contained in:
+101
-1
@@ -17,7 +17,11 @@ import { sidePanelNavigationMorphItemsByPageState } from '@/side-panel/states/si
|
||||
import { activeTabIdComponentState } from '@/ui/layout/tab-list/states/activeTabIdComponentState';
|
||||
import { useAtomComponentStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomComponentStateValue';
|
||||
import { jotaiStore } from '@/ui/utilities/state/jotai/jotaiStore';
|
||||
import { ContextStorePageType, SidePanelPages } from 'twenty-shared/types';
|
||||
import {
|
||||
AppPath,
|
||||
ContextStorePageType,
|
||||
SidePanelPages,
|
||||
} from 'twenty-shared/types';
|
||||
import { useIcons } from 'twenty-ui/icon';
|
||||
import { PageLayoutType } from '~/generated-metadata/graphql';
|
||||
import { getJestMetadataAndApolloMocksAndCommandMenuWrapper } from '~/testing/jest/getJestMetadataAndApolloMocksAndCommandMenuWrapper';
|
||||
@@ -45,6 +49,17 @@ jest.mock(
|
||||
}),
|
||||
);
|
||||
|
||||
const mockNavigateApp = jest.fn();
|
||||
jest.mock('~/hooks/useNavigateApp', () => ({
|
||||
useNavigateApp: () => mockNavigateApp,
|
||||
}));
|
||||
|
||||
let mockIsMobile = false;
|
||||
jest.mock('twenty-ui/utilities', () => ({
|
||||
...jest.requireActual('twenty-ui/utilities'),
|
||||
useIsMobile: () => mockIsMobile,
|
||||
}));
|
||||
|
||||
const personMockObjectMetadataItem =
|
||||
getTestEnrichedObjectMetadataItemsMock().find(
|
||||
(item) => item.nameSingular === 'person',
|
||||
@@ -117,6 +132,7 @@ const renderHooks = () => {
|
||||
describe('useOpenRecordInSidePanel', () => {
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
mockIsMobile = false;
|
||||
});
|
||||
|
||||
it('should set the correct states and navigate to the record page', () => {
|
||||
@@ -256,4 +272,88 @@ describe('useOpenRecordInSidePanel', () => {
|
||||
|
||||
expect(mockOpenNewRecordTitleCell).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should navigate to the record page instead of the side panel on mobile', () => {
|
||||
mockIsMobile = true;
|
||||
const { result } = renderHooks();
|
||||
|
||||
act(() => {
|
||||
result.current.openRecordInSidePanel({
|
||||
recordId: 'record-123',
|
||||
objectNameSingular: 'person',
|
||||
});
|
||||
});
|
||||
|
||||
expect(mockNavigateApp).toHaveBeenCalledWith(
|
||||
AppPath.RecordShowPage,
|
||||
{ objectNameSingular: 'person', objectRecordId: 'record-123' },
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
expect(mockNavigateSidePanel).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should forward new record state to the record page on mobile', () => {
|
||||
mockIsMobile = true;
|
||||
const { result } = renderHooks();
|
||||
|
||||
act(() => {
|
||||
result.current.openRecordInSidePanel({
|
||||
recordId: 'new-record-123',
|
||||
objectNameSingular: 'person',
|
||||
isNewRecord: true,
|
||||
});
|
||||
});
|
||||
|
||||
expect(mockNavigateApp).toHaveBeenCalledWith(
|
||||
AppPath.RecordShowPage,
|
||||
{ objectNameSingular: 'person', objectRecordId: 'new-record-123' },
|
||||
undefined,
|
||||
{
|
||||
state: {
|
||||
isNewRecord: true,
|
||||
objectRecordId: 'new-record-123',
|
||||
labelIdentifierFieldName: getLabelIdentifierFieldMetadataItem(
|
||||
personMockObjectMetadataItem,
|
||||
)?.name,
|
||||
},
|
||||
},
|
||||
);
|
||||
expect(mockOpenNewRecordTitleCell).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should preset the record page active tab on mobile', () => {
|
||||
mockIsMobile = true;
|
||||
const { result } = renderHooks();
|
||||
|
||||
const recordId = 'record-123';
|
||||
const objectNameSingular = 'person';
|
||||
|
||||
act(() => {
|
||||
result.current.openRecordInSidePanel({
|
||||
recordId,
|
||||
objectNameSingular,
|
||||
tab: 'tab-emails',
|
||||
});
|
||||
});
|
||||
|
||||
const tabListInstanceId = getTabListInstanceIdFromPageLayoutAndRecord({
|
||||
pageLayoutId: getDefaultRecordPageLayoutId({
|
||||
targetObjectNameSingular: objectNameSingular,
|
||||
}),
|
||||
layoutType: PageLayoutType.RECORD_PAGE,
|
||||
targetRecordIdentifier: {
|
||||
id: recordId,
|
||||
targetObjectNameSingular: objectNameSingular,
|
||||
},
|
||||
});
|
||||
|
||||
expect(
|
||||
jotaiStore.get(
|
||||
activeTabIdComponentState.atomFamily({
|
||||
instanceId: tabListInstanceId,
|
||||
}),
|
||||
),
|
||||
).toBe('tab-emails');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,6 +16,7 @@ import { viewableRecordIdState } from '@/object-record/record-side-panel/states/
|
||||
import { useOpenNewRecordTitleCell } from '@/object-record/record-title-cell/hooks/useOpenNewRecordTitleCell';
|
||||
import { setRecordPageActiveTabId } from '@/page-layout/utils/setRecordPageActiveTabId';
|
||||
import {
|
||||
AppPath,
|
||||
ContextStorePageType,
|
||||
CoreObjectNameSingular,
|
||||
SidePanelPages,
|
||||
@@ -27,17 +28,22 @@ import { useStore } from 'jotai';
|
||||
import { useCallback } from 'react';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { useIcons } from 'twenty-ui/icon';
|
||||
import { useIsMobile } from 'twenty-ui/utilities';
|
||||
import { v4 } from 'uuid';
|
||||
import { useNavigateApp } from '~/hooks/useNavigateApp';
|
||||
|
||||
export const useOpenRecordInSidePanel = () => {
|
||||
const store = useStore();
|
||||
const { getIcon } = useIcons();
|
||||
|
||||
const { navigateSidePanelMenu } = useSidePanelMenu();
|
||||
const { navigateSidePanelMenu, closeSidePanelMenu } = useSidePanelMenu();
|
||||
const { runWorkflowRunOpeningInSidePanelEffects } =
|
||||
useRunWorkflowRunOpeningInSidePanelEffects();
|
||||
const { openNewRecordTitleCell } = useOpenNewRecordTitleCell();
|
||||
|
||||
const isMobile = useIsMobile();
|
||||
const navigate = useNavigateApp();
|
||||
|
||||
const openRecordInSidePanel = useCallback(
|
||||
({
|
||||
recordId,
|
||||
@@ -61,6 +67,38 @@ export const useOpenRecordInSidePanel = () => {
|
||||
});
|
||||
}
|
||||
|
||||
if (isMobile) {
|
||||
const objectMetadataItemForRecordPage = store.get(
|
||||
objectMetadataItemFamilySelector.selectorFamily({
|
||||
objectName: objectNameSingular,
|
||||
objectNameType: 'singular',
|
||||
}),
|
||||
);
|
||||
|
||||
const labelIdentifierField = isDefined(objectMetadataItemForRecordPage)
|
||||
? getLabelIdentifierFieldMetadataItem(objectMetadataItemForRecordPage)
|
||||
: undefined;
|
||||
|
||||
closeSidePanelMenu();
|
||||
|
||||
navigate(
|
||||
AppPath.RecordShowPage,
|
||||
{ objectNameSingular, objectRecordId: recordId },
|
||||
undefined,
|
||||
isNewRecord
|
||||
? {
|
||||
state: {
|
||||
isNewRecord: true,
|
||||
objectRecordId: recordId,
|
||||
labelIdentifierFieldName: labelIdentifierField?.name,
|
||||
},
|
||||
}
|
||||
: undefined,
|
||||
);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
const navigationStack = store.get(sidePanelNavigationStackState.atom);
|
||||
|
||||
const currentNavigationStackItem = navigationStack.at(-1);
|
||||
@@ -206,7 +244,10 @@ export const useOpenRecordInSidePanel = () => {
|
||||
}
|
||||
},
|
||||
[
|
||||
closeSidePanelMenu,
|
||||
getIcon,
|
||||
isMobile,
|
||||
navigate,
|
||||
navigateSidePanelMenu,
|
||||
openNewRecordTitleCell,
|
||||
runWorkflowRunOpeningInSidePanelEffects,
|
||||
|
||||
Reference in New Issue
Block a user