Fix: prevent record navigation when clicking Remove from favorite in nav sidebar (#18760)

https://github.com/user-attachments/assets/96abf04d-726a-4225-846e-e5d701a583a2 --------- Co-authored-by: Charles Bochet <charles@twenty.com>
2026-03-19 19:59:12 +05:30
parent 5526d2e5d0
commit a9f8a7e1fa
7 changed files with 83 additions and 134 deletions
@@ -40,8 +40,7 @@ export const SettingsNavigationDrawerItem = ({
          to={href}
          Icon={item.Icon}
          active={isActive}
-          soon={item.soon}
-          isNew={item.isNew}
+          modifier={item.modifier}
          onClick={item.onClick}
        />
      </AdvancedSettingsWrapper>
@@ -56,8 +55,7 @@ export const SettingsNavigationDrawerItem = ({
      to={href || undefined}
      Icon={item.Icon}
      active={isActive}
-      soon={item.soon}
-      isNew={item.isNew}
+      modifier={item.modifier}
      onClick={item.onClick}
    />
  );
@@ -7,7 +7,10 @@ import { billingState } from '@/client-config/states/billingState';
 import { supportChatState } from '@/client-config/states/supportChatState';
 import { usePermissionFlagMap } from '@/settings/roles/hooks/usePermissionFlagMap';
 import { getDocumentationUrl } from '@/support/utils/getDocumentationUrl';
-import { type NavigationDrawerItemIndentationLevel } from '@/ui/navigation/navigation-drawer/components/NavigationDrawerItem';
+import {
+  type NavigationDrawerItemIndentationLevel,
+  type NavigationDrawerItemModifier,
+} from '@/ui/navigation/navigation-drawer/components/NavigationDrawerItem';
 import { useAtomStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomStateValue';
 import { useIsFeatureEnabled } from '@/workspace/hooks/useIsFeatureEnabled';
 import { t } from '@lingui/core/macro';
@@ -57,8 +60,7 @@ export type SettingsNavigationItem = {
  isHidden?: boolean;
  subItems?: SettingsNavigationItem[];
  isAdvanced?: boolean;
-  soon?: boolean;
-  isNew?: boolean;
+  modifier?: NavigationDrawerItemModifier;
 };

 const useSettingsNavigationItems = (): SettingsNavigationSection[] => {
@@ -179,7 +181,7 @@ const useSettingsNavigationItems = (): SettingsNavigationSection[] => {
          isHidden:
            !isApplicationEnabled ||
            !permissionMap[PermissionFlagType.WORKSPACE],
-          isNew: true,
+          modifier: 'new',
        },
        {
          label: t`AI`,
@@ -187,7 +189,7 @@ const useSettingsNavigationItems = (): SettingsNavigationSection[] => {
          Icon: IconSparkles,
          isHidden:
            !isAIEnabled || !permissionMap[PermissionFlagType.WORKSPACE],
-          isNew: true,
+          modifier: 'new',
        },
        {
          label: t`Security`,
@@ -45,7 +45,7 @@ export const SignInAppNavigationDrawerMock = ({
              label={t`Search`}
              Icon={IconSearch}
              onClick={() => {}}
-              keyboard={[getOsControlSymbol(), 'K']}
+              modifier={{ keyboard: [getOsControlSymbol(), 'K'] }}
            />
            <NavigationDrawerItem
              label={t`Settings`}
@@ -37,6 +37,11 @@ const DEFAULT_INDENTATION_LEVEL = 1;

 export type NavigationDrawerItemIndentationLevel = 1 | 2;

+export type NavigationDrawerItemModifier =
+  | 'soon'
+  | 'new'
+  | { keyboard: string[] };
+
 export type NavigationDrawerItemProps = {
  className?: string;
  label: string;
@@ -48,17 +53,12 @@ export type NavigationDrawerItemProps = {
  Icon?: IconComponent | ((props: TablerIconsProps) => JSX.Element);
  iconColor?: string | null;
  active?: boolean;
-  danger?: boolean;
-  soon?: boolean;
-  isNew?: boolean;
-  count?: number;
-  keyboard?: string[];
+  modifier?: NavigationDrawerItemModifier;
  rightOptions?: ReactNode;
  alwaysShowRightOptions?: boolean;
  isDragging?: boolean;
  isRightOptionsDropdownOpen?: boolean;
  triggerEvent?: TriggerEventType;
-  mouseUpNavigation?: boolean;
  preventCollapseOnMobile?: boolean;
  isSelectedInEditMode?: boolean;
  variant?: 'default' | 'tertiary';
@@ -67,14 +67,13 @@ export type NavigationDrawerItemProps = {
 type StyledItemProps = Pick<
  NavigationDrawerItemProps,
  | 'active'
-  | 'danger'
  | 'indentationLevel'
-  | 'soon'
  | 'to'
  | 'isDragging'
  | 'isSelectedInEditMode'
  | 'variant'
 > & {
+  isSoon: boolean;
  isNavigationDrawerExpanded: boolean;
  hasRightOptions: boolean;
  href?: string;
@@ -92,14 +91,11 @@ const StyledItem = styled.button<StyledItemProps>`
      : '1px solid transparent'};
  border-radius: ${themeCssVariables.border.radius.sm};
  box-sizing: border-box;
-  color: ${({ active, danger, soon, variant }) => {
+  color: ${({ active, isSoon, variant }) => {
    if (active === true) {
      return themeCssVariables.font.color.primary;
    }
-    if (danger === true) {
-      return themeCssVariables.color.red;
-    }
-    if (soon === true) {
+    if (isSoon) {
      return themeCssVariables.font.color.light;
    }
    if (variant === 'tertiary') {
@@ -107,8 +103,8 @@ const StyledItem = styled.button<StyledItemProps>`
    }
    return themeCssVariables.font.color.secondary;
  }};
-  cursor: ${({ soon, isDragging }) =>
-    isDragging ? 'grabbing' : soon ? 'default' : 'pointer'};
+  cursor: ${({ isSoon, isDragging }) =>
+    isDragging ? 'grabbing' : isSoon ? 'default' : 'pointer'};
  display: flex;
  font-family: ${themeCssVariables.font.family};
  font-size: ${themeCssVariables.font.size.md};
@@ -122,7 +118,7 @@ const StyledItem = styled.button<StyledItemProps>`
      ? themeCssVariables.spacing['0.5']
      : themeCssVariables.spacing[1]};
  padding-top: ${themeCssVariables.spacing[1]};
-  pointer-events: ${({ soon }) => (soon ? 'none' : 'auto')};
+  pointer-events: ${({ isSoon }) => (isSoon ? 'none' : 'auto')};
  text-decoration: none;
  user-select: none;
  width: ${({ isNavigationDrawerExpanded, hasRightOptions }) =>
@@ -132,10 +128,7 @@ const StyledItem = styled.button<StyledItemProps>`

  &:hover {
    background: ${themeCssVariables.background.transparent.light};
-    color: ${({ danger }) =>
-      danger
-        ? themeCssVariables.color.red
-        : themeCssVariables.font.color.primary};
+    color: ${themeCssVariables.font.color.primary};
  }

  &:hover .keyboard-shortcuts {
@@ -172,20 +165,6 @@ const StyledItemSecondaryLabel = styled.span`
  font-weight: ${themeCssVariables.font.weight.regular};
 `;

-const StyledItemCount = styled.span`
-  align-items: center;
-  background-color: ${themeCssVariables.color.blue};
-  border-radius: ${themeCssVariables.border.radius.rounded};
-  color: ${themeCssVariables.grayScale.gray1};
-  display: flex;
-  font-size: ${themeCssVariables.font.size.xs};
-  font-weight: ${themeCssVariables.font.weight.semiBold};
-  height: 16px;
-  justify-content: center;
-  margin-left: auto;
-  width: 16px;
-`;
-
 const StyledKeyBoardShortcut = styled.span`
  align-items: center;
  background: ${themeCssVariables.background.transparent.lighter};
@@ -277,18 +256,13 @@ export const NavigationDrawerItem = ({
  to,
  onClick,
  active,
-  danger,
-  soon,
-  isNew,
-  count,
-  keyboard,
+  modifier,
  subItemState,
  rightOptions,
  alwaysShowRightOptions = false,
  isDragging,
  isRightOptionsDropdownOpen,
  triggerEvent,
-  mouseUpNavigation = false,
  preventCollapseOnMobile = false,
  isSelectedInEditMode = false,
  variant = 'default',
@@ -301,10 +275,15 @@ export const NavigationDrawerItem = ({

  const { navigationItemId } = useNavigationDrawerTooltip(label, to);

+  const isSoon = modifier === 'soon';
+  const isNew = modifier === 'new';
+  const keyboardKeys =
+    isDefined(modifier) && typeof modifier === 'object'
+      ? modifier.keyboard
+      : undefined;
+
  const showBreadcrumb = indentationLevel === 2;
-  const showStyledSpacer = Boolean(
-    soon || isNew || count || keyboard || rightOptions,
-  );
+  const showStyledSpacer = isDefined(modifier) || isDefined(rightOptions);

  const handleMobileNavigation = () => {
    if (isMobile && !preventCollapseOnMobile) {
@@ -314,6 +293,7 @@ export const NavigationDrawerItem = ({

  const isExternalLink =
    isDefined(to) && (to.startsWith('http://') || to.startsWith('https://'));
+  const isInternalLink = isDefined(to) && !isExternalLink;

  const handleExternalLinkClick = () => {
    handleMobileNavigation();
@@ -332,40 +312,37 @@ export const NavigationDrawerItem = ({
    triggerEvent,
  });

+  const elementType = isExternalLink
+    ? 'a'
+    : isInternalLink
+      ? Link
+      : isDefined(rightOptions)
+        ? 'div'
+        : undefined;
+
  return (
    <StyledNavigationDrawerItemContainer>
      <StyledItem
        id={navigationItemId}
        className={`navigation-drawer-item ${className || ''}`}
-        onClick={
-          mouseUpNavigation ? onClick : handleMouseDownNavigationClickClick
-        }
-        onMouseDown={mouseUpNavigation ? undefined : handleMouseDown}
+        onClick={handleMouseDownNavigationClickClick}
+        onMouseDown={handleMouseDown}
        active={active}
        aria-selected={active}
-        danger={danger}
-        soon={soon}
+        isSoon={isSoon}
        variant={variant}
-        as={
-          to
-            ? isExternalLink
-              ? 'a'
-              : Link
-            : isDefined(rightOptions)
-              ? 'div'
-              : undefined
-        }
-        role={to ? undefined : isDefined(rightOptions) ? 'button' : undefined}
-        to={isExternalLink ? undefined : to}
-        href={isExternalLink ? to : undefined}
-        target={isExternalLink ? '_blank' : undefined}
-        rel={isExternalLink ? 'noopener noreferrer' : undefined}
-        draggable={to && !isExternalLink ? false : undefined}
        indentationLevel={indentationLevel}
        isNavigationDrawerExpanded={isNavigationDrawerExpanded}
        isDragging={isDragging}
        hasRightOptions={isDefined(rightOptions)}
        isSelectedInEditMode={isSelectedInEditMode}
+        as={elementType}
+        role={!to && isDefined(rightOptions) ? 'button' : undefined}
+        to={isInternalLink ? to : undefined}
+        href={isExternalLink ? to : undefined}
+        target={isExternalLink ? '_blank' : undefined}
+        rel={isExternalLink ? 'noopener noreferrer' : undefined}
+        draggable={isInternalLink ? false : undefined}
      >
        <StyledItemElementsContainer>
          {showBreadcrumb && (
@@ -419,7 +396,7 @@ export const NavigationDrawerItem = ({

          {showStyledSpacer && <StyledSpacer />}

-          {soon && (
+          {isSoon && (
            <NavigationDrawerAnimatedCollapseWrapper>
              <Pill label={t`Soon`} />
            </NavigationDrawerAnimatedCollapseWrapper>
@@ -431,23 +408,25 @@ export const NavigationDrawerItem = ({
            </NavigationDrawerAnimatedCollapseWrapper>
          )}

-          {!!count && (
-            <NavigationDrawerAnimatedCollapseWrapper>
-              <StyledItemCount>{count}</StyledItemCount>
-            </NavigationDrawerAnimatedCollapseWrapper>
-          )}
-
-          {keyboard && (
+          {isDefined(keyboardKeys) && (
            <NavigationDrawerAnimatedCollapseWrapper>
              <StyledKeyBoardShortcut className="keyboard-shortcuts">
-                <Label>{keyboard}</Label>
+                <Label>{keyboardKeys}</Label>
              </StyledKeyBoardShortcut>
            </NavigationDrawerAnimatedCollapseWrapper>
          )}

          {isDefined(rightOptions) && (
            <NavigationDrawerAnimatedCollapseWrapper>
-              <StyledRightOptionsContainer>
+              {/* When StyledItem renders as a Link, we need both handlers to
+                  prevent navigation when interacting with rightOptions:
+                  - onMouseDown: stops useMouseDownNavigation from calling navigate()
+                  - onClickCapture: prevents the native <a> follow since the child's
+                    stopPropagation blocks Link's own preventDefault */}
+              <StyledRightOptionsContainer
+                onMouseDown={(e) => e.stopPropagation()}
+                onClickCapture={(e) => e.preventDefault()}
+              >
                <StyledRightOptionsVisbility
                  data-visible={
                    isMobile ||
@@ -14,10 +14,7 @@ export const NavigationDrawerSubItem = ({
  to,
  onClick,
  active,
-  danger,
-  soon,
-  count,
-  keyboard,
+  modifier,
  subItemState,
  rightOptions,
  isDragging,
@@ -37,10 +34,7 @@ export const NavigationDrawerSubItem = ({
      to={to}
      onClick={onClick}
      active={active}
-      danger={danger}
-      soon={soon}
-      count={count}
-      keyboard={keyboard}
+      modifier={modifier}
      rightOptions={rightOptions}
      isDragging={isDragging}
      isSelectedInEditMode={isSelectedInEditMode}
@@ -86,24 +86,19 @@ export const Default: Story = {
            label="Notifications"
            to="/inbox"
            Icon={IconBell}
-            soon={true}
+            modifier="soon"
          />
          <NavigationDrawerItem
            label="Search"
            Icon={IconSearch}
-            keyboard={[`${getOsControlSymbol()}`, 'K']}
+            modifier={{ keyboard: [`${getOsControlSymbol()}`, 'K'] }}
          />
          <NavigationDrawerItem
            label="Settings"
            to="/settings/profile"
            Icon={IconSettings}
          />
-          <NavigationDrawerItem
-            label="Tasks"
-            to="/tasks"
-            Icon={IconCheckbox}
-            count={2}
-          />
+          <NavigationDrawerItem label="Tasks" to="/tasks" Icon={IconCheckbox} />
        </NavigationDrawerSection>

        <NavigationDrawerSection>
@@ -105,23 +105,14 @@ export const NewPill: Story = {
          args={{
            label: 'New Feature',
            Icon: IconSearch,
-            isNew: true,
+            modifier: 'new',
          }}
        />
        <Story
          args={{
-            label: 'New Feature with Count',
+            label: 'Feature with Keyboard Shortcut',
            Icon: IconSearch,
-            isNew: true,
-            count: 5,
-          }}
-        />
-        <Story
-          args={{
-            label: 'New Feature with Keyboard Shortcut',
-            Icon: IconSearch,
-            isNew: true,
-            keyboard: [getOsControlSymbol(), 'N'],
+            modifier: { keyboard: [getOsControlSymbol(), 'N'] },
          }}
        />
      </StyledContainer>
@@ -195,12 +186,6 @@ export const Catalog: CatalogStory<Story, typeof NavigationDrawerItem> = {
    pseudo: { hover: ['.hover'] },
    catalog: {
      dimensions: [
-        {
-          name: 'danger',
-          values: [true, false],
-          props: (danger: boolean) => ({ danger }),
-          labels: (danger: boolean) => (danger ? 'Danger' : 'No Danger'),
-        },
        {
          name: 'active',
          values: [true, false],
@@ -216,23 +201,19 @@ export const Catalog: CatalogStory<Story, typeof NavigationDrawerItem> = {
        },
        {
          name: 'adornments',
-          values: [
-            'Without Adornments',
-            'Soon Pill',
-            'New Pill',
-            'Count',
-            'Keyboard Keys',
-          ],
+          values: ['Without Modifier', 'Soon', 'New', 'Keyboard Keys'],
          props: (adornmentName: string) =>
-            adornmentName === 'Soon Pill'
-              ? { soon: true }
-              : adornmentName === 'New Pill'
-                ? { isNew: true }
-                : adornmentName === 'Count'
-                  ? { count: 3 }
-                  : adornmentName === 'Keyboard Keys'
-                    ? { keyboard: [getOsControlSymbol(), 'K'] }
-                    : {},
+            adornmentName === 'Soon'
+              ? { modifier: 'soon' }
+              : adornmentName === 'New'
+                ? { modifier: 'new' }
+                : adornmentName === 'Keyboard Keys'
+                  ? {
+                      modifier: {
+                        keyboard: [getOsControlSymbol(), 'K'],
+                      },
+                    }
+                  : {},
        },
      ],
    },