Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ export const SaveDashboardSingleRecordAction = () => {
const { setIsDashboardInEditMode } =
useSetIsDashboardInEditMode(pageLayoutId);

const handleClick = () => {
savePageLayout();
const handleClick = async () => {
await savePageLayout();
setIsDashboardInEditMode(false);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { PageLayoutGridLayout } from '@/page-layout/components/PageLayoutGridLayout';
import { useCreatePageLayoutTab } from '@/page-layout/hooks/useCreatePageLayoutTab';
import { useCurrentPageLayout } from '@/page-layout/hooks/useCurrentPageLayout';
import { isPageLayoutInEditModeComponentState } from '@/page-layout/states/isPageLayoutInEditModeComponentState';
import { getTabListInstanceIdFromPageLayoutId } from '@/page-layout/utils/getTabListInstanceIdFromPageLayoutId';
import { TabList } from '@/ui/layout/tab-list/components/TabList';
import { ScrollWrapper } from '@/ui/utilities/scroll/components/ScrollWrapper';
import { useRecoilComponentValue } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValue';
import styled from '@emotion/styled';
import { isDefined } from 'twenty-shared/utils';

Expand All @@ -26,6 +29,14 @@ const StyledScrollWrapper = styled(ScrollWrapper)`
export const PageLayoutRendererContent = () => {
const { currentPageLayout } = useCurrentPageLayout();

const isPageLayoutInEditMode = useRecoilComponentValue(
isPageLayoutInEditModeComponentState,
);

const { createPageLayoutTab } = useCreatePageLayoutTab(currentPageLayout?.id);

const handleAddTab = isPageLayoutInEditMode ? createPageLayoutTab : undefined;

if (!isDefined(currentPageLayout)) {
return null;
}
Expand All @@ -38,6 +49,7 @@ export const PageLayoutRendererContent = () => {
componentInstanceId={getTabListInstanceIdFromPageLayoutId(
currentPageLayout.id,
)}
onAddTab={handleAddTab}
/>
<StyledScrollWrapper
componentInstanceId={`scroll-wrapper-page-layout-${currentPageLayout.id}`}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import { pageLayoutCurrentLayoutsComponentState } from '@/page-layout/states/pageLayoutCurrentLayoutsComponentState';
import { pageLayoutDraftComponentState } from '@/page-layout/states/pageLayoutDraftComponentState';
import { activeTabIdComponentState } from '@/ui/layout/tab-list/states/activeTabIdComponentState';
import { useRecoilComponentValue } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValue';
import { useSetRecoilComponentState } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentState';
import { act, renderHook } from '@testing-library/react';
import { useSetRecoilState } from 'recoil';
import { PageLayoutType } from '~/generated/graphql';
import { useCreatePageLayoutTab } from '../useCreatePageLayoutTab';
import {
PageLayoutTestWrapper,
PAGE_LAYOUT_TEST_INSTANCE_ID,
PageLayoutTestWrapper,
} from './PageLayoutTestWrapper';

jest.mock('uuid', () => ({
Expand All @@ -20,44 +24,49 @@ describe('useCreatePageLayoutTab', () => {
it('should create a new tab with default title', () => {
const uuidModule = require('uuid');
uuidModule.v4.mockReturnValue('mock-uuid');

const { result } = renderHook(
() => ({
createTab: useCreatePageLayoutTab(PAGE_LAYOUT_TEST_INSTANCE_ID),
pageLayoutDraft: useRecoilComponentValue(
pageLayoutDraftComponentState,
PAGE_LAYOUT_TEST_INSTANCE_ID,
),
pageLayoutCurrentLayouts: useRecoilComponentValue(
pageLayoutCurrentLayoutsComponentState,
PAGE_LAYOUT_TEST_INSTANCE_ID,
),
pageLayoutDraft: useRecoilComponentValue(
pageLayoutDraftComponentState,
PAGE_LAYOUT_TEST_INSTANCE_ID,
activeTabId: useSetRecoilState(
activeTabIdComponentState.atomFamily({
instanceId: `${PAGE_LAYOUT_TEST_INSTANCE_ID}-tab-list`,
}),
),
}),
{
wrapper: PageLayoutTestWrapper,
},
);

let newTabId: string;
act(() => {
newTabId = result.current.createTab.createPageLayoutTab();
result.current.createTab.createPageLayoutTab();
});

expect(result.current.pageLayoutDraft.tabs[0].id).toBe('tab-mock-uuid');
expect(result.current.pageLayoutDraft.tabs).toHaveLength(1);
expect(result.current.pageLayoutDraft.tabs[0].id).toBe('mock-uuid');
expect(result.current.pageLayoutDraft.tabs[0].title).toBe('Tab 1');
expect(result.current.pageLayoutDraft.tabs[0].position).toBe(0);
expect(result.current.pageLayoutDraft.tabs[0].widgets).toEqual([]);

expect(result.current.pageLayoutCurrentLayouts['tab-mock-uuid']).toEqual({
expect(result.current.pageLayoutCurrentLayouts['mock-uuid']).toEqual({
desktop: [],
mobile: [],
});

expect(newTabId!).toBe('tab-mock-uuid');
});

it('should create a new tab with custom title', () => {
const uuidModule = require('uuid');
uuidModule.v4.mockReturnValue('mock-uuid');

const { result } = renderHook(
() => ({
createTab: useCreatePageLayoutTab(PAGE_LAYOUT_TEST_INSTANCE_ID),
Expand All @@ -83,8 +92,9 @@ describe('useCreatePageLayoutTab', () => {
it('should increment position for subsequent tabs', () => {
const uuidModule = require('uuid');
uuidModule.v4
.mockReturnValueOnce('mock-uuid')
.mockReturnValueOnce('mock-uuid-1')
.mockReturnValueOnce('mock-uuid-2');

const { result } = renderHook(
() => ({
createTab: useCreatePageLayoutTab(PAGE_LAYOUT_TEST_INSTANCE_ID),
Expand All @@ -107,8 +117,10 @@ describe('useCreatePageLayoutTab', () => {
});

expect(result.current.pageLayoutDraft.tabs).toHaveLength(2);
expect(result.current.pageLayoutDraft.tabs[0].id).toBe('mock-uuid-1');
expect(result.current.pageLayoutDraft.tabs[0].position).toBe(0);
expect(result.current.pageLayoutDraft.tabs[0].title).toBe('Tab 1');
expect(result.current.pageLayoutDraft.tabs[1].id).toBe('mock-uuid-2');
expect(result.current.pageLayoutDraft.tabs[1].position).toBe(1);
expect(result.current.pageLayoutDraft.tabs[1].title).toBe('Tab 2');
});
Expand All @@ -118,6 +130,7 @@ describe('useCreatePageLayoutTab', () => {
uuidModule.v4
.mockReturnValueOnce('mock-uuid-1')
.mockReturnValueOnce('mock-uuid-2');

const { result } = renderHook(
() => ({
createTab: useCreatePageLayoutTab(PAGE_LAYOUT_TEST_INSTANCE_ID),
Expand All @@ -131,24 +144,109 @@ describe('useCreatePageLayoutTab', () => {
},
);

let tabId1: string = '';
act(() => {
tabId1 = result.current.createTab.createPageLayoutTab();
result.current.createTab.createPageLayoutTab();
});

let tabId2: string = '';
act(() => {
tabId2 = result.current.createTab.createPageLayoutTab();
result.current.createTab.createPageLayoutTab();
});

expect(result.current.pageLayoutCurrentLayouts[tabId1]).toEqual({
const tabIds = Object.keys(result.current.pageLayoutCurrentLayouts);
expect(tabIds).toHaveLength(2);
expect(tabIds).toContain('mock-uuid-1');
expect(tabIds).toContain('mock-uuid-2');

expect(result.current.pageLayoutCurrentLayouts['mock-uuid-1']).toEqual({
desktop: [],
mobile: [],
});
expect(result.current.pageLayoutCurrentLayouts[tabId2]).toEqual({
expect(result.current.pageLayoutCurrentLayouts['mock-uuid-2']).toEqual({
desktop: [],
mobile: [],
});
expect(tabId1).not.toBe(tabId2);
expect(tabIds[0]).not.toBe(tabIds[1]);
});

it('should set newly created tab as active', () => {
const uuidModule = require('uuid');
uuidModule.v4.mockReturnValue('mock-uuid');

const { result } = renderHook(
() => {
const getActiveTabId = useRecoilComponentValue(
activeTabIdComponentState,
`${PAGE_LAYOUT_TEST_INSTANCE_ID}-tab-list`,
);
return {
createTab: useCreatePageLayoutTab(PAGE_LAYOUT_TEST_INSTANCE_ID),
activeTabId: getActiveTabId,
};
},
{
wrapper: PageLayoutTestWrapper,
},
);

expect(result.current.activeTabId).toBeNull();

act(() => {
result.current.createTab.createPageLayoutTab();
});

expect(result.current.activeTabId).toBe('mock-uuid');
});

it('should handle creating tab when draft already has tabs', () => {
const uuidModule = require('uuid');
uuidModule.v4.mockReturnValue('mock-uuid-new');

const { result } = renderHook(
() => {
const setPageLayoutDraft = useSetRecoilComponentState(
pageLayoutDraftComponentState,
PAGE_LAYOUT_TEST_INSTANCE_ID,
);
const pageLayoutDraft = useRecoilComponentValue(
pageLayoutDraftComponentState,
PAGE_LAYOUT_TEST_INSTANCE_ID,
);
const createTab = useCreatePageLayoutTab(PAGE_LAYOUT_TEST_INSTANCE_ID);
return { setPageLayoutDraft, pageLayoutDraft, createTab };
},
{
wrapper: PageLayoutTestWrapper,
},
);

act(() => {
result.current.setPageLayoutDraft({
id: 'test-layout',
name: 'Test Layout',
type: PageLayoutType.DASHBOARD,
objectMetadataId: null,
tabs: [
{
id: 'existing-tab',
title: 'Existing Tab',
position: 0,
pageLayoutId: 'test-layout',
widgets: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
deletedAt: null,
},
],
});
});

act(() => {
result.current.createTab.createPageLayoutTab();
});

expect(result.current.pageLayoutDraft.tabs).toHaveLength(2);
expect(result.current.pageLayoutDraft.tabs[1].id).toBe('mock-uuid-new');
expect(result.current.pageLayoutDraft.tabs[1].position).toBe(1);
expect(result.current.pageLayoutDraft.tabs[1].title).toBe('Tab 2');
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { PageLayoutComponentInstanceContext } from '@/page-layout/states/contexts/PageLayoutComponentInstanceContext';
import { getTabListInstanceIdFromPageLayoutId } from '@/page-layout/utils/getTabListInstanceIdFromPageLayoutId';
import { activeTabIdComponentState } from '@/ui/layout/tab-list/states/activeTabIdComponentState';
import { useAvailableComponentInstanceIdOrThrow } from '@/ui/utilities/state/component-state/hooks/useAvailableComponentInstanceIdOrThrow';
import { useRecoilComponentCallbackState } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentCallbackState';
import { useSetRecoilComponentState } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentState';
import { useRecoilCallback } from 'recoil';
import { v4 as uuidv4 } from 'uuid';
import { pageLayoutCurrentLayoutsComponentState } from '../states/pageLayoutCurrentLayoutsComponentState';
Expand All @@ -24,20 +27,26 @@ export const useCreatePageLayoutTab = (pageLayoutIdFromProps?: string) => {
pageLayoutId,
);

const tabListInstanceId = getTabListInstanceIdFromPageLayoutId(pageLayoutId);
const setActiveTabId = useSetRecoilComponentState(
activeTabIdComponentState,
tabListInstanceId,
);

const createPageLayoutTab = useRecoilCallback(
({ snapshot, set }) =>
(title?: string): string => {
(title?: string): void => {
const pageLayoutDraft = snapshot
.getLoadable(pageLayoutDraftState)
.getValue();

const newTabId = `tab-${uuidv4()}`;
const newTabId = uuidv4();
const tabsLength = pageLayoutDraft.tabs.length;
const newTab: PageLayoutTab = {
id: newTabId,
title: title || `Tab ${tabsLength + 1}`,
position: tabsLength,
pageLayoutId: '',
pageLayoutId: pageLayoutId,
widgets: [],
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
Expand All @@ -55,9 +64,14 @@ export const useCreatePageLayoutTab = (pageLayoutIdFromProps?: string) => {
getEmptyTabLayout(prev, newTabId),
);

return newTabId;
setActiveTabId(newTabId);
},
[pageLayoutCurrentLayoutsState, pageLayoutDraftState],
[
pageLayoutCurrentLayoutsState,
pageLayoutDraftState,
pageLayoutId,
setActiveTabId,
],
);

return { createPageLayoutTab };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ export const transformPageLayout = (
): PageLayout => {
return {
...pageLayout,
tabs: (pageLayout.tabs ?? []).map(
(tab): PageLayoutTab => ({
...tab,
widgets: tab.widgets ?? [],
}),
),
tabs: (pageLayout.tabs ?? [])
.toSorted((a, b) => a.position - b.position)
.map(
(tab): PageLayoutTab => ({
...tab,
widgets: tab.widgets ?? [],
}),
),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -218,12 +218,7 @@ export const TabList = ({
{onAddTab && (
<NodeDimension onDimensionChange={handleAddButtonWidthChange}>
<StyledAddButton>
<IconButton
Icon={IconPlus}
size="small"
variant="tertiary"
onClick={onAddTab}
/>
<IconButton Icon={IconPlus} size="small" variant="tertiary" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: The add button in the hidden measurement container is missing the onClick handler while the visible one has it

Suggested change
<IconButton Icon={IconPlus} size="small" variant="tertiary" />
<IconButton Icon={IconPlus} size="small" variant="tertiary" onClick={() => onAddTab()} />
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/twenty-front/src/modules/ui/layout/tab-list/components/TabList.tsx
Line: 221:221

Comment:
syntax: The add button in the hidden measurement container is missing the onClick handler while the visible one has it

```suggestion
                  <IconButton Icon={IconPlus} size="small" variant="tertiary" onClick={() => onAddTab()} />
```

How can I resolve this? If you propose a fix, please make it concise.

</StyledAddButton>
</NodeDimension>
)}
Expand Down Expand Up @@ -274,7 +269,7 @@ export const TabList = ({
Icon={IconPlus}
size="small"
variant="tertiary"
onClick={onAddTab}
onClick={() => onAddTab()}
/>
</StyledAddButton>
)}
Expand Down