From 4e59f00e3fbbb646066c4e3e80cf2eac77a4aeb0 Mon Sep 17 00:00:00 2001 From: Balaji Krishnamurthy <107975017+BKM14@users.noreply.github.com> Date: Thu, 24 Oct 2024 17:50:45 +0530 Subject: [PATCH] Use 'role' = button for chip navigation (#8011) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #7817 Added role attribute to the div element of the Chip component. This assigns the role of "button" to the container, which is important for accessibility. It indicates that this div should be treated as a button by assistive technologies like screen readers. --------- Co-authored-by: Félix Malfait Co-authored-by: Félix Malfait --- .../object-record/components/RecordChip.tsx | 24 +++++++---------- .../components/RecordIndexRecordChip.tsx | 8 ++---- .../contexts/RecordIndexRootPropsContext.ts | 2 +- .../hooks/useHandleIndexIdentifierClick.ts | 9 +++---- .../hooks/useOpenRecordTableCellV2.ts | 10 ++++--- .../SettingsNavigationDrawerItems.tsx | 2 +- .../pages/object-record/RecordIndexPage.tsx | 4 +-- .../display/chip/components/AvatarChip.tsx | 27 ++++++++++++++++--- 8 files changed, 49 insertions(+), 37 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/components/RecordChip.tsx b/packages/twenty-front/src/modules/object-record/components/RecordChip.tsx index 5e073040f..6a7a51e4b 100644 --- a/packages/twenty-front/src/modules/object-record/components/RecordChip.tsx +++ b/packages/twenty-front/src/modules/object-record/components/RecordChip.tsx @@ -1,4 +1,4 @@ -import { AvatarChip, AvatarChipVariant, UndecoratedLink } from 'twenty-ui'; +import { AvatarChip, AvatarChipVariant } from 'twenty-ui'; import { getLinkToShowPage } from '@/object-metadata/utils/getLinkToShowPage'; import { useRecordChipData } from '@/object-record/hooks/useRecordChipData'; @@ -23,24 +23,20 @@ export const RecordChip = ({ record, }); - const handleClick = (e: MouseEvent) => { + const handleClick = (e: MouseEvent) => { e.stopPropagation(); }; return ( - - {}} - /> - + /> ); }; diff --git a/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexRecordChip.tsx b/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexRecordChip.tsx index d883ce55f..fbeddb158 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexRecordChip.tsx +++ b/packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexRecordChip.tsx @@ -18,16 +18,12 @@ export const RecordIdentifierChip = ({ variant, size, }: RecordIdentifierChipProps) => { - const { onIndexIdentifierClick } = useContext(RecordIndexRootPropsContext); + const { indexIdentifierUrl } = useContext(RecordIndexRootPropsContext); const { recordChipData } = useRecordChipData({ objectNameSingular, record, }); - const handleAvatarChipClick = () => { - onIndexIdentifierClick(record.id); - }; - const { Icon: LeftIcon, IconColor: LeftIconColor } = useGetStandardObjectIcon(objectNameSingular); return ( @@ -36,7 +32,7 @@ export const RecordIdentifierChip = ({ name={recordChipData.name} avatarType={recordChipData.avatarType} avatarUrl={recordChipData.avatarUrl ?? ''} - onClick={handleAvatarChipClick} + to={indexIdentifierUrl(record.id)} variant={variant} LeftIcon={LeftIcon} LeftIconColor={LeftIconColor} diff --git a/packages/twenty-front/src/modules/object-record/record-index/contexts/RecordIndexRootPropsContext.ts b/packages/twenty-front/src/modules/object-record/record-index/contexts/RecordIndexRootPropsContext.ts index 1546fd30a..267501019 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/contexts/RecordIndexRootPropsContext.ts +++ b/packages/twenty-front/src/modules/object-record/record-index/contexts/RecordIndexRootPropsContext.ts @@ -2,7 +2,7 @@ import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; import { createRootPropsContext } from '~/utils/createRootPropsContext'; export type RecordIndexRootPropsContextProps = { - onIndexIdentifierClick: (recordId: string) => void; + indexIdentifierUrl: (recordId: string) => string; onIndexRecordsLoaded: () => void; onCreateRecord: () => void; objectNamePlural: string; diff --git a/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleIndexIdentifierClick.ts b/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleIndexIdentifierClick.ts index 2ea5bcdc4..9a17c838f 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleIndexIdentifierClick.ts +++ b/packages/twenty-front/src/modules/object-record/record-index/hooks/useHandleIndexIdentifierClick.ts @@ -2,7 +2,6 @@ import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; import { buildShowPageURL } from '@/object-record/record-show/utils/buildShowPageURL'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; import { currentViewIdComponentState } from '@/views/states/currentViewIdComponentState'; -import { useNavigate } from 'react-router-dom'; export const useHandleIndexIdentifierClick = ({ objectMetadataItem, @@ -11,22 +10,20 @@ export const useHandleIndexIdentifierClick = ({ recordIndexId: string; objectMetadataItem: ObjectMetadataItem; }) => { - const navigate = useNavigate(); - const currentViewId = useRecoilComponentValueV2( currentViewIdComponentState, recordIndexId, ); - const handleIndexIdentifierClick = (recordId: string) => { + const indexIdentifierUrl = (recordId: string) => { const showPageURL = buildShowPageURL( objectMetadataItem.nameSingular, recordId, currentViewId, ); - navigate(showPageURL); + return showPageURL; }; - return { handleIndexIdentifierClick }; + return { indexIdentifierUrl }; }; diff --git a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellV2.ts b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellV2.ts index 0159b907b..f3d522e1f 100644 --- a/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellV2.ts +++ b/packages/twenty-front/src/modules/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCellV2.ts @@ -22,6 +22,7 @@ import { isDefined } from '~/utils/isDefined'; import { RecordIndexRootPropsContext } from '@/object-record/record-index/contexts/RecordIndexRootPropsContext'; import { useContext } from 'react'; +import { useNavigate } from 'react-router-dom'; import { TableHotkeyScope } from '../../types/TableHotkeyScope'; export const DEFAULT_CELL_SCOPE: HotkeyScope = { @@ -41,7 +42,7 @@ export type OpenTableCellArgs = { }; export const useOpenRecordTableCellV2 = (tableScopeId: string) => { - const { onIndexIdentifierClick } = useContext(RecordIndexRootPropsContext); + const { indexIdentifierUrl } = useContext(RecordIndexRootPropsContext); const moveEditModeToTableCellPosition = useMoveEditModeToTableCellPosition(tableScopeId); @@ -61,6 +62,8 @@ export const useOpenRecordTableCellV2 = (tableScopeId: string) => { viewableRecordNameSingularState, ); + const navigate = useNavigate(); + const openTableCell = useRecoilCallback( ({ snapshot }) => ({ @@ -95,7 +98,7 @@ export const useOpenRecordTableCellV2 = (tableScopeId: string) => { if (isFirstColumnCell && !isEmpty && !isActionButtonClick) { leaveTableFocus(); - onIndexIdentifierClick(recordId); + navigate(indexIdentifierUrl(recordId)); return; } @@ -143,7 +146,8 @@ export const useOpenRecordTableCellV2 = (tableScopeId: string) => { openRightDrawer, setViewableRecordId, setViewableRecordNameSingular, - onIndexIdentifierClick, + indexIdentifierUrl, + navigate, ], ); diff --git a/packages/twenty-front/src/modules/settings/components/SettingsNavigationDrawerItems.tsx b/packages/twenty-front/src/modules/settings/components/SettingsNavigationDrawerItems.tsx index 8f191abf6..f762df6e8 100644 --- a/packages/twenty-front/src/modules/settings/components/SettingsNavigationDrawerItems.tsx +++ b/packages/twenty-front/src/modules/settings/components/SettingsNavigationDrawerItems.tsx @@ -10,13 +10,13 @@ import { IconDoorEnter, IconFunction, IconHierarchy2, + IconKey, IconMail, IconRocket, IconSettings, IconTool, IconUserCircle, IconUsers, - IconKey, MAIN_COLORS, } from 'twenty-ui'; diff --git a/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx b/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx index c9352d52f..c15536d8c 100644 --- a/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx +++ b/packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx @@ -44,7 +44,7 @@ export const RecordIndexPage = () => { createNewTableRecord(); }; - const { handleIndexIdentifierClick } = useHandleIndexIdentifierClick({ + const { indexIdentifierUrl } = useHandleIndexIdentifierClick({ objectMetadataItem, recordIndexId, }); @@ -67,7 +67,7 @@ export const RecordIndexPage = () => { objectNameSingular, objectMetadataItem, onIndexRecordsLoaded: handleIndexRecordsLoaded, - onIndexIdentifierClick: handleIndexIdentifierClick, + indexIdentifierUrl, onCreateRecord: handleCreateRecord, }} > diff --git a/packages/twenty-ui/src/display/chip/components/AvatarChip.tsx b/packages/twenty-ui/src/display/chip/components/AvatarChip.tsx index d71afa2f6..32aec0420 100644 --- a/packages/twenty-ui/src/display/chip/components/AvatarChip.tsx +++ b/packages/twenty-ui/src/display/chip/components/AvatarChip.tsx @@ -8,6 +8,9 @@ import { isDefined } from '@ui/utilities/isDefined'; import { Nullable } from '@ui/utilities/types/Nullable'; import { MouseEvent, useContext } from 'react'; +// Import Link from react-router-dom instead of UndecoratedLink +import { Link } from 'react-router-dom'; + export type AvatarChipProps = { name: string; avatarUrl?: string; @@ -20,6 +23,7 @@ export type AvatarChipProps = { className?: string; placeholderColorSeed?: string; onClick?: (event: MouseEvent) => void; + to?: string; }; export enum AvatarChipVariant { @@ -37,6 +41,12 @@ const StyledInvertedIconContainer = styled.div<{ backgroundColor: string }>` background-color: ${({ backgroundColor }) => backgroundColor}; `; +// Ideally we would use the UndecoratedLink component from @ui/navigation +// but it led to a bug probably linked to circular dependencies, which was hard to solve +const StyledLink = styled(Link)` + text-decoration: none; +`; + export const AvatarChip = ({ name, avatarUrl, @@ -48,15 +58,16 @@ export const AvatarChip = ({ className, placeholderColorSeed, onClick, + to, size = ChipSize.Small, }: AvatarChipProps) => { const { theme } = useContext(ThemeContext); - return ( + const chip = ( ) } - clickable={isDefined(onClick)} - onClick={onClick} + clickable={isDefined(onClick) || isDefined(to)} + onClick={to ? undefined : onClick} className={className} /> ); + + return to ? ( + + {chip} + + ) : ( + chip + ); };