Skip to content
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
5a3d585
WIP
ehconitin May 30, 2025
124d17d
wip
ehconitin May 30, 2025
c2f9cd7
copy over tab styles to button
ehconitin May 30, 2025
9b70fa3
refactor
ehconitin May 30, 2025
182fecb
refactor
ehconitin May 30, 2025
ca31855
update story books
ehconitin May 30, 2025
dc35ac1
continue
ehconitin May 30, 2025
f939fc9
continue
ehconitin May 30, 2025
8662165
continue + lint
ehconitin May 30, 2025
7111bc0
do not have a function returning
ehconitin May 30, 2025
2770136
Merge branch 'main' into new-tab-list
ehconitin May 30, 2025
50bd701
Merge branch 'main' into new-tab-list
ehconitin Jun 2, 2025
6513944
charles review
ehconitin Jun 3, 2025
1e3950d
storybook fix?
ehconitin Jun 3, 2025
51f69b1
Merge branch 'main' into new-tab-list
ehconitin Jun 3, 2025
812d132
fixed?
ehconitin Jun 3, 2025
e5cf25d
is this standard?
ehconitin Jun 3, 2025
5e608ff
this is wrong, reverting -- need to rethink this part
ehconitin Jun 3, 2025
cb231a9
Merge branch 'main' into new-tab-list
ehconitin Jun 4, 2025
da8afdf
yagni - we did not need forward ref in tab
ehconitin Jun 4, 2025
25ab03b
refactor: use ID-based dictionary for tab width tracking
ehconitin Jun 4, 2025
07cc564
remove redundant useMemo
ehconitin Jun 4, 2025
26fc018
simplify api
ehconitin Jun 4, 2025
7d1f378
revert right paddings
ehconitin Jun 4, 2025
c56e8f9
revert right paddings
ehconitin Jun 4, 2025
9ef1352
add TabButton to twenty-ui and refactor
ehconitin Jun 5, 2025
3f95c62
Merge branch 'main' into new-tab-list
ehconitin Jun 5, 2025
a45b29f
add disable test id prop to tabbutton
ehconitin Jun 5, 2025
a769611
remove seeds
ehconitin Jun 5, 2025
cd9f36a
refacto
ehconitin Jun 5, 2025
9756cae
Merge branch 'main' into new-tab-list
ehconitin Jun 5, 2025
6677c3e
Merge branch 'main' into new-tab-list
charlesBochet Jun 5, 2025
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 @@ -3,7 +3,6 @@ import { getRightDrawerActionMenuDropdownIdFromActionMenuId } from '@/action-men
import { useCommandMenu } from '@/command-menu/hooks/useCommandMenu';
import { CommandMenuPageComponentInstanceContext } from '@/command-menu/states/contexts/CommandMenuPageComponentInstanceContext';
import { CoreObjectNameSingular } from '@/object-metadata/types/CoreObjectNameSingular';
import { getLinkToShowPage } from '@/object-metadata/utils/getLinkToShowPage';
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
import { ObjectRecord } from '@/object-record/types/ObjectRecord';
import { AppPath } from '@/types/AppPath';
Expand All @@ -16,18 +15,13 @@ import { useAvailableComponentInstanceIdOrThrow } from '@/ui/utilities/state/com
import { useComponentInstanceStateContext } from '@/ui/utilities/state/component-state/hooks/useComponentInstanceStateContext';
import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';
import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentStateV2';
import styled from '@emotion/styled';
import { useCallback } from 'react';
import { Link } from 'react-router-dom';
import { useRecoilValue } from 'recoil';
import { isDefined } from 'twenty-shared/utils';
import { IconBrowserMaximize } from 'twenty-ui/display';
import { Button } from 'twenty-ui/input';
import { getOsControlSymbol } from 'twenty-ui/utilities';
import { useNavigateApp } from '~/hooks/useNavigateApp';
const StyledLink = styled(Link)`
text-decoration: none;
`;

type RecordShowRightDrawerOpenRecordButtonProps = {
objectNameSingular: string;
Expand Down Expand Up @@ -117,18 +111,15 @@ export const RecordShowRightDrawerOpenRecordButton = ({
return null;
}

const to = getLinkToShowPage(objectNameSingular, record);

return (
<StyledLink to={to} onClick={closeCommandMenu}>
<Button
title="Open"
variant="primary"
accent="blue"
size="medium"
Icon={IconBrowserMaximize}
hotkeys={[getOsControlSymbol(), '⏎']}
/>
</StyledLink>
<Button
title="Open"
variant="primary"
accent="blue"
size="medium"
Icon={IconBrowserMaximize}
hotkeys={[getOsControlSymbol(), '⏎']}
onClick={handleOpenRecord}
/>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const StyledContainer = styled.div`
const StyledTabList = styled(TabList)`
background-color: ${({ theme }) => theme.background.secondary};
padding-left: ${({ theme }) => theme.spacing(2)};
padding-right: ${({ theme }) => theme.spacing(2)};
Copy link
Member

Choose a reason for hiding this comment

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

@thomtrp does this makes sense to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see any change 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, this is redundant now
before the more button always used to render the extreme right end of the tablist -- flex: 1
The idea was to have similar padding on both ends -- which wasn't required when we had a scroll.
However, now that the requirements have changed, this feels redundant.
Thanks for pointing it out!!
new requirements - #12384 (comment)

`;

type TabId = WorkflowRunTabIdType;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,54 @@ export const SettingsAdminContent = () => {
Icon: IconHeart,
disabled: !canAccessFullAdminPanel,
},
{
id: SETTINGS_ADMIN_TABS.CHECK_1,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

temporary seeds

title: t`C`,
Icon: IconHeart,
disabled: !canAccessFullAdminPanel,
},
{
id: SETTINGS_ADMIN_TABS.CHECK_2,
title: t`Check 2`,
Icon: IconHeart,
disabled: !canAccessFullAdminPanel,
},
{
id: SETTINGS_ADMIN_TABS.CHECK_3,
title: t`Check 3`,
Icon: IconHeart,
disabled: !canAccessFullAdminPanel,
},
{
id: SETTINGS_ADMIN_TABS.CHECK_4,
title: t`Check 4`,
Icon: IconHeart,
disabled: !canAccessFullAdminPanel,
},
{
id: SETTINGS_ADMIN_TABS.CHECK_5,
title: t`Check 5`,
Icon: IconHeart,
disabled: !canAccessFullAdminPanel,
},
{
id: SETTINGS_ADMIN_TABS.CHECK_6,
title: t`Check 6`,
Icon: IconHeart,
disabled: !canAccessFullAdminPanel,
},
{
id: SETTINGS_ADMIN_TABS.CHECK_7,
title: t`Check 7`,
Icon: IconHeart,
disabled: !canAccessFullAdminPanel,
},
{
id: SETTINGS_ADMIN_TABS.CHECK_8,
title: t`Check 8`,
Icon: IconHeart,
disabled: !canAccessFullAdminPanel,
},
];

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,22 @@ export const SettingsAdminTabContent = () => {
return <SettingsAdminConfigVariables />;
case SETTINGS_ADMIN_TABS.HEALTH_STATUS:
return <SettingsAdminHealthStatus />;
case SETTINGS_ADMIN_TABS.CHECK_1:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same

return <div>Check 1</div>;
case SETTINGS_ADMIN_TABS.CHECK_2:
return <div>Check 2</div>;
case SETTINGS_ADMIN_TABS.CHECK_3:
return <div>Check 3</div>;
case SETTINGS_ADMIN_TABS.CHECK_4:
return <div>Check 4</div>;
case SETTINGS_ADMIN_TABS.CHECK_5:
return <div>Check 5</div>;
case SETTINGS_ADMIN_TABS.CHECK_6:
return <div>Check 6</div>;
case SETTINGS_ADMIN_TABS.CHECK_7:
return <div>Check 7</div>;
case SETTINGS_ADMIN_TABS.CHECK_8:
return <div>Check 8</div>;
default:
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,12 @@ export const SETTINGS_ADMIN_TABS = {
GENERAL: 'general',
CONFIG_VARIABLES: 'config-variables',
HEALTH_STATUS: 'health-status',
CHECK_1: 'check-1',
CHECK_2: 'check-2',
CHECK_3: 'check-3',
CHECK_4: 'check-4',
CHECK_5: 'check-5',
CHECK_6: 'check-6',
CHECK_7: 'check-7',
CHECK_8: 'check-8',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same

};
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,18 @@ const StyledShowPageRightContainer = styled.div<{ isMobile: boolean }>`
`;

const StyledTabListContainer = styled.div<{ shouldDisplay: boolean }>`
display: ${({ shouldDisplay }) => (shouldDisplay ? 'flex' : 'none')};
${({ shouldDisplay }) =>
Copy link
Member

Choose a reason for hiding this comment

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

question: not against it but what is this change about? this section is subject to regressions due to show page vs right drawer vs mobile, let's check all of them

!shouldDisplay &&
`
visibility: hidden;
height: 0;
overflow: hidden;
`}
`;

const StyledTabList = styled(TabList)`
padding-left: ${({ theme }) => theme.spacing(2)};
padding-right: ${({ theme }) => theme.spacing(2)};
`;

const StyledContentContainer = styled.div<{ isInRightDrawer: boolean }>`
Expand Down
186 changes: 71 additions & 115 deletions packages/twenty-front/src/modules/ui/layout/tab/components/Tab.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
import { EllipsisDisplay } from '@/ui/field/display/components/EllipsisDisplay';
import isPropValid from '@emotion/is-prop-valid';
import { useTheme } from '@emotion/react';
import styled from '@emotion/styled';
import { ReactElement } from 'react';
import { ReactElement, forwardRef } from 'react';
import { Link } from 'react-router-dom';
import { Pill } from 'twenty-ui/components';
import { Avatar, IconComponent } from 'twenty-ui/display';
import { useMouseDownNavigation } from 'twenty-ui/utilities';
import {
StyledTabBase,
StyledTabHover,
StyledTabIconContainer,
} from './TabSharedStyles';

type TabProps = {
id: string;
Expand All @@ -21,119 +23,73 @@ type TabProps = {
logo?: string;
};

const StyledTab = styled('button', {
shouldForwardProp: (prop) => isPropValid(prop) && prop !== 'active',
})<{ active?: boolean; disabled?: boolean; to?: string }>`
all: unset;
align-items: center;
border-bottom: 1px solid ${({ theme }) => theme.border.color.light};
border-color: ${({ theme, active }) =>
active ? theme.border.color.inverted : 'transparent'};
color: ${({ theme, active, disabled }) =>
active
export const Tab = forwardRef<HTMLButtonElement, TabProps>(
(
{
id,
title,
Icon,
active = false,
onClick,
className,
disabled,
pill,
to,
logo,
},
ref,
) => {
const theme = useTheme();
const { onClick: handleClick, onMouseDown: handleMouseDown } =
useMouseDownNavigation({
to,
onClick,
disabled,
});

const iconColor = active
? theme.font.color.primary
: disabled
? theme.font.color.light
: theme.font.color.secondary};
cursor: pointer;
background-color: transparent;
border-left: none;
border-right: none;
border-top: none;
font-family: inherit;

display: flex;
gap: ${({ theme }) => theme.spacing(1)};
justify-content: center;
margin-bottom: -1px;
padding: ${({ theme }) => theme.spacing(2) + ' 0'};
pointer-events: ${({ disabled }) => (disabled ? 'none' : '')};
text-decoration: none;
`;

const StyledHover = styled.span`
display: flex;
gap: ${({ theme }) => theme.spacing(1)};
padding: ${({ theme }) => theme.spacing(1)};
padding-left: ${({ theme }) => theme.spacing(2)};
padding-right: ${({ theme }) => theme.spacing(2)};
font-weight: ${({ theme }) => theme.font.weight.medium};
width: 100%;
&:hover {
background: ${({ theme }) => theme.background.tertiary};
border-radius: ${({ theme }) => theme.border.radius.sm};
}
&:active {
background: ${({ theme }) => theme.background.quaternary};
}
`;
: theme.font.color.secondary;

const StyledIconContainer = styled.div`
flex-shrink: 0;
display: flex;
align-items: center;
justify-content: center;
`;
return (
<StyledTabBase
ref={ref}
onClick={handleClick}
onMouseDown={handleMouseDown}
active={active}
className={className}
disabled={disabled}
data-testid={'tab-' + id}
as={to ? Link : 'button'}
to={to}
>
<StyledTabHover>
<StyledTabIconContainer>
{logo && (
<Avatar
avatarUrl={logo}
size="md"
placeholder={title}
iconColor={iconColor}
/>
)}
{Icon && (
<Avatar
Icon={Icon}
size="md"
placeholder={title}
iconColor={iconColor}
/>
)}
</StyledTabIconContainer>
{title}
{pill && typeof pill === 'string' ? <Pill label={pill} /> : pill}
</StyledTabHover>
</StyledTabBase>
);
},
);

export const Tab = ({
id,
title,
Icon,
active = false,
onClick,
className,
disabled,
pill,
to,
logo,
}: TabProps) => {
const theme = useTheme();
const { onClick: handleClick, onMouseDown: handleMouseDown } =
useMouseDownNavigation({
to,
onClick,
disabled,
});

const iconColor = active
? theme.font.color.primary
: disabled
? theme.font.color.light
: theme.font.color.secondary;

return (
<StyledTab
onClick={handleClick}
onMouseDown={handleMouseDown}
active={active}
className={className}
disabled={disabled}
data-testid={'tab-' + id}
as={to ? Link : 'button'}
to={to}
>
<StyledHover>
<StyledIconContainer>
{logo && (
<Avatar
avatarUrl={logo}
size="md"
placeholder={title}
iconColor={iconColor}
/>
)}
{Icon && (
<Avatar
Icon={Icon}
size="md"
placeholder={title}
iconColor={iconColor}
/>
)}
</StyledIconContainer>
<EllipsisDisplay>{title}</EllipsisDisplay>
{pill && typeof pill === 'string' ? <Pill label={pill} /> : pill}
</StyledHover>
</StyledTab>
);
};
Tab.displayName = 'Tab';
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to add this here? Is it because of the forwardRef?
Seems non standard to me but we can revisit if there is a good reason

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes forward ref! Not related to this PR.
pretty sure this was added just for clear debugging in devtools

Loading
Loading