Fix command menu keyboard input & refactor group (#1706)

* - fix command menu keyboard shortcuts - refactor command groups * - refactor the MenuItemCommand to use cmdk * - fixed matching commands multiple displays * - fixed array count problems react with boolean
2023-09-22 11:44:42 +02:00
parent 8d8c81c02c
commit 20267f081a
5 changed files with 130 additions and 158 deletions
--- a/front/src/modules/command-menu/components/CommandGroup.tsx
+++ b/front/src/modules/command-menu/components/CommandGroup.tsx
@ -0,0 +1,31 @@
+import React from 'react';
+import styled from '@emotion/styled';
+import { Command } from 'cmdk';
+
+const StyledGroup = styled(Command.Group)`
+  [cmdk-group-heading] {
+    align-items: center;
+    color: ${({ theme }) => theme.font.color.light};
+    display: flex;
+    font-size: ${({ theme }) => theme.font.size.xs};
+    font-weight: ${({ theme }) => theme.font.weight.semiBold};
+    padding-bottom: ${({ theme }) => theme.spacing(2)};
+    padding-left: ${({ theme }) => theme.spacing(2)};
+    padding-right: ${({ theme }) => theme.spacing(1)};
+    padding-top: ${({ theme }) => theme.spacing(2)};
+    text-transform: uppercase;
+    user-select: none;
+  }
+`;
+
+type OwnProps = {
+  heading: string;
+  children: React.ReactNode | React.ReactNode[];
+};
+
+export const CommandGroup = ({ heading, children }: OwnProps) => {
+  if (!children || !React.Children.count(children)) {
+    return null;
+  }
+  return <StyledGroup heading={heading}>{children}</StyledGroup>;
+};
--- a/front/src/modules/command-menu/components/CommandMenu.tsx
+++ b/front/src/modules/command-menu/components/CommandMenu.tsx
@ -19,11 +19,11 @@ import { commandMenuCommandsState } from '../states/commandMenuCommandsState';
 import { isCommandMenuOpenedState } from '../states/isCommandMenuOpenedState';
 import { Command, CommandType } from '../types/Command';

+import { CommandGroup } from './CommandGroup';
 import { CommandMenuItem } from './CommandMenuItem';
 import {
  StyledDialog,
  StyledEmpty,
-  StyledGroup,
  StyledInput,
  StyledList,
 } from './CommandMenuStyles';
@ -130,14 +130,10 @@ export const CommandMenu = () => {
        onValueChange={setSearch}
      />
      <StyledList>
-        {matchingCreateCommand.length < 1 &&
-          matchingNavigateCommand.length < 1 &&
-          people.length < 1 &&
-          companies.length < 1 &&
-          activities.length < 1 && <StyledEmpty>No results found.</StyledEmpty>}
-        {matchingCreateCommand.length > 0 && (
-          <StyledGroup heading="Create">
-            {matchingCreateCommand.map((cmd) => (
+        <StyledEmpty>No results found.</StyledEmpty>
+        <CommandGroup heading="Create">
+          {matchingCreateCommand.length === 1 &&
+            matchingCreateCommand.map((cmd) => (
              <CommandMenuItem
                to={cmd.to}
                key={cmd.label}
@ -147,11 +143,10 @@ export const CommandMenu = () => {
                shortcuts={cmd.shortcuts || []}
              />
            ))}
-          </StyledGroup>
-        )}
-        {matchingNavigateCommand.length > 0 && (
-          <StyledGroup heading="Navigate">
-            {matchingNavigateCommand.map((cmd) => (
+        </CommandGroup>
+        <CommandGroup heading="Navigate">
+          {matchingNavigateCommand.length === 1 &&
+            matchingNavigateCommand.map((cmd) => (
              <CommandMenuItem
                to={cmd.to}
                key={cmd.label}
@ -160,57 +155,50 @@ export const CommandMenu = () => {
                shortcuts={cmd.shortcuts || []}
              />
            ))}
-          </StyledGroup>
-        )}
-        {people.length > 0 && (
-          <StyledGroup heading="People">
-            {people.map((person) => (
-              <CommandMenuItem
-                key={person.id}
-                to={`person/${person.id}`}
-                label={person.displayName}
-                Icon={() => (
-                  <Avatar
-                    type="rounded"
-                    avatarUrl={null}
-                    colorId={person.id}
-                    placeholder={person.displayName}
-                  />
-                )}
-              />
-            ))}
-          </StyledGroup>
-        )}
-        {companies.length > 0 && (
-          <StyledGroup heading="Companies">
-            {companies.map((company) => (
-              <CommandMenuItem
-                key={company.id}
-                label={company.name}
-                to={`companies/${company.id}`}
-                Icon={() => (
-                  <Avatar
-                    colorId={company.id}
-                    placeholder={company.name}
-                    avatarUrl={getLogoUrlFromDomainName(company.domainName)}
-                  />
-                )}
-              />
-            ))}
-          </StyledGroup>
-        )}
-        {activities.length > 0 && (
-          <StyledGroup heading="Notes">
-            {activities.map((activity) => (
-              <CommandMenuItem
-                Icon={IconNotes}
-                key={activity.id}
-                label={activity.title ?? ''}
-                onClick={() => openActivityRightDrawer(activity.id)}
-              />
-            ))}
-          </StyledGroup>
-        )}
+        </CommandGroup>
+        <CommandGroup heading="People">
+          {people.map((person) => (
+            <CommandMenuItem
+              key={person.id}
+              to={`person/${person.id}`}
+              label={person.displayName}
+              Icon={() => (
+                <Avatar
+                  type="rounded"
+                  avatarUrl={null}
+                  colorId={person.id}
+                  placeholder={person.displayName}
+                />
+              )}
+            />
+          ))}
+        </CommandGroup>
+        <CommandGroup heading="Companies">
+          {companies.map((company) => (
+            <CommandMenuItem
+              key={company.id}
+              label={company.name}
+              to={`companies/${company.id}`}
+              Icon={() => (
+                <Avatar
+                  colorId={company.id}
+                  placeholder={company.name}
+                  avatarUrl={getLogoUrlFromDomainName(company.domainName)}
+                />
+              )}
+            />
+          ))}
+        </CommandGroup>
+        <CommandGroup heading="Notes">
+          {activities.map((activity) => (
+            <CommandMenuItem
+              Icon={IconNotes}
+              key={activity.id}
+              label={activity.title ?? ''}
+              onClick={() => openActivityRightDrawer(activity.id)}
+            />
+          ))}
+        </CommandGroup>
      </StyledList>
    </StyledDialog>
  );
--- a/front/src/modules/command-menu/components/CommandMenuItem.tsx
+++ b/front/src/modules/command-menu/components/CommandMenuItem.tsx
@ -1,4 +1,3 @@
-import React from 'react';
 import { useNavigate } from 'react-router-dom';

 import { IconArrowUpRight } from '@/ui/icon';
--- a/front/src/modules/command-menu/components/CommandMenuStyles.tsx
+++ b/front/src/modules/command-menu/components/CommandMenuStyles.tsx
@ -37,48 +37,6 @@ export const StyledInput = styled(Command.Input)`
  }
 `;

-export const StyledMenuItem = styled(Command.Item)`
-  align-items: center;
-  border-radius: ${({ theme }) => theme.border.radius.sm};
-  color: ${({ theme }) => theme.font.color.primary};
-  cursor: pointer;
-  display: flex;
-  font-size: ${({ theme }) => theme.font.size.md};
-  gap: ${({ theme }) => theme.spacing(3)};
-  height: 40px;
-  justify-content: space-between;
-  padding: 0 ${({ theme }) => theme.spacing(1)};
-  position: relative;
-  transition: all 150ms ease;
-  transition-property: none;
-  user-select: none;
-  &:hover {
-    background: ${({ theme }) => theme.background.transparent.light};
-  }
-  &[data-selected='true'] {
-    background: ${({ theme }) => theme.background.tertiary};
-    /* Could be nice to add a caret like this for better accessibility in the future
-    But it needs to be consistend with other picker dropdown (e.g. company)
-    &:after {
-      background: ${({ theme }) => theme.background.quaternary};
-      content: '';
-      height: 100%;
-      left: 0;
-      position: absolute;
-      width: 3px;
-      z-index: ${({ theme }) => theme.lastLayerZIndex};
-    } */
-  }
-  &[data-disabled='true'] {
-    color: ${({ theme }) => theme.font.color.light};
-    cursor: not-allowed;
-  }
-  svg {
-    height: 16px;
-    width: 16px;
-  }
-`;
-
 export const StyledList = styled(Command.List)`
  background: ${({ theme }) => theme.background.secondary};
  height: min(300px, var(--cmdk-list-height));
@ -89,22 +47,6 @@ export const StyledList = styled(Command.List)`
  transition-property: height;
 `;

-export const StyledGroup = styled(Command.Group)`
-  [cmdk-group-heading] {
-    align-items: center;
-    color: ${({ theme }) => theme.font.color.light};
-    display: flex;
-    font-size: ${({ theme }) => theme.font.size.xs};
-    font-weight: ${({ theme }) => theme.font.weight.semiBold};
-    padding-bottom: ${({ theme }) => theme.spacing(2)};
-    padding-left: ${({ theme }) => theme.spacing(2)};
-    padding-right: ${({ theme }) => theme.spacing(1)};
-    padding-top: ${({ theme }) => theme.spacing(2)};
-    text-transform: uppercase;
-    user-select: none;
-  }
-`;
-
 export const StyledEmpty = styled(Command.Empty)`
  align-items: center;
  color: ${({ theme }) => theme.font.color.light};
@ -114,34 +56,3 @@ export const StyledEmpty = styled(Command.Empty)`
  justify-content: center;
  white-space: pre-wrap;
 `;
-
-export const StyledSeparator = styled(Command.Separator)``;
-
-export const StyledIconAndLabelContainer = styled.div`
-  align-items: center;
-  display: flex;
-  gap: ${({ theme }) => theme.spacing(2)};
-`;
-export const StyledIconContainer = styled.div`
-  align-items: center;
-  background: ${({ theme }) => theme.background.transparent.light};
-  border-radius: ${({ theme }) => theme.border.radius.sm};
-  color: ${({ theme }) => theme.font.color.secondary};
-  display: flex;
-  padding: ${({ theme }) => theme.spacing(1)};
-`;
-export const StyledShortCut = styled.div`
-  background-color: ${({ theme }) => theme.background.transparent.light};
-  border-radius: ${({ theme }) => theme.border.radius.sm};
-  color: ${({ theme }) => theme.font.color.light};
-  margin-left: ${({ theme }) => theme.spacing(1)};
-  margin-right: ${({ theme }) => theme.spacing(1)};
-  padding: ${({ theme }) => theme.spacing(1)};
-`;
-
-export const StyledShortcutsContainer = styled.div`
-  align-items: center;
-  color: ${({ theme }) => theme.font.color.light};
-  display: flex;
-  font-size: ${({ theme }) => theme.font.size.sm};
-`;
--- a/front/src/modules/ui/menu-item/components/MenuItemCommand.tsx
+++ b/front/src/modules/ui/menu-item/components/MenuItemCommand.tsx
@ -1,10 +1,10 @@
 import { useTheme } from '@emotion/react';
 import styled from '@emotion/styled';
+import { Command } from 'cmdk';

 import { IconComponent } from '@/ui/icon/types/IconComponent';

 import {
-  StyledMenuItemBase,
  StyledMenuItemLabel,
  StyledMenuItemLeftContent,
 } from '../internals/components/StyledMenuItemBase';
@ -32,8 +32,51 @@ const StyledCommandText = styled.div`
  padding-right: ${({ theme }) => theme.spacing(2)};
 `;

-const StyledMenuItemCommandContainer = styled(StyledMenuItemBase)`
+const StyledMenuItemCommandContainer = styled(Command.Item)`
+  --horizontal-padding: ${({ theme }) => theme.spacing(1)};
+  --vertical-padding: ${({ theme }) => theme.spacing(2)};
+  align-items: center;
+  border-radius: ${({ theme }) => theme.border.radius.sm};
+  color: ${({ theme }) => theme.font.color.secondary};
+  cursor: pointer;
+  display: flex;
+  flex-direction: row;
+  font-size: ${({ theme }) => theme.font.size.sm};
+  gap: ${({ theme }) => theme.spacing(2)};
+  height: calc(32px - 2 * var(--vertical-padding));
  height: 24px;
+  justify-content: space-between;
+  padding: var(--vertical-padding) var(--horizontal-padding);
+  position: relative;
+  transition: all 150ms ease;
+  transition-property: none;
+  user-select: none;
+  width: calc(100% - 2 * var(--horizontal-padding));
+  &:hover {
+    background: ${({ theme }) => theme.background.transparent.light};
+  }
+  &[data-selected='true'] {
+    background: ${({ theme }) => theme.background.tertiary};
+    /* Could be nice to add a caret like this for better accessibility in the future
+    But it needs to be consistend with other picker dropdown (e.g. company)
+    &:after {
+      background: ${({ theme }) => theme.background.quaternary};
+      content: '';
+      height: 100%;
+      left: 0;
+      position: absolute;
+      width: 3px;
+      z-index: ${({ theme }) => theme.lastLayerZIndex};
+    } */
+  }
+  &[data-disabled='true'] {
+    color: ${({ theme }) => theme.font.color.light};
+    cursor: not-allowed;
+  }
+  svg {
+    height: 16px;
+    width: 16px;
+  }
 `;

 export type MenuItemProps = {
@ -54,7 +97,7 @@ export const MenuItemCommand = ({
  const theme = useTheme();

  return (
-    <StyledMenuItemCommandContainer onClick={onClick} className={className}>
+    <StyledMenuItemCommandContainer onSelect={onClick} className={className}>
      <StyledMenuItemLeftContent>
        {LeftIcon && (
          <StyledBigIconContainer>