feat - Compact sidebar #7414

ehconitin · 2024-10-03T11:40:04Z

fixes #7143
@Bonapara

2024-10-03.17-04-29.mp4

Recent updates

demo.mov

To Do

fix breadcrumb

github-actions · 2024-10-03T11:41:02Z

	Fails
🚫	`node` failed.

Log

�[31mError: �[39m RequestError [HttpError]: You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 4C04:D0C2B:4268D8D:7F5CD35:670E5A06.
    at /home/runner/work/twenty/twenty/node_modules/�[4m@octokit�[24m/request/dist-node/index.js:86:21
�[90m    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)�[39m {
  status: �[33m403�[39m,
  response: {
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3Aehconitin%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    status: �[33m403�[39m,
    headers: {
      �[32m'access-control-allow-origin'�[39m: �[32m'*'�[39m,
      �[32m'access-control-expose-headers'�[39m: �[32m'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset'�[39m,
      connection: �[32m'close'�[39m,
      �[32m'content-encoding'�[39m: �[32m'gzip'�[39m,
      �[32m'content-security-policy'�[39m: �[32m"default-src 'none'"�[39m,
      �[32m'content-type'�[39m: �[32m'application/json; charset=utf-8'�[39m,
      date: �[32m'Tue, 15 Oct 2024 12:03:18 GMT'�[39m,
      �[32m'referrer-policy'�[39m: �[32m'origin-when-cross-origin, strict-origin-when-cross-origin'�[39m,
      server: �[32m'github.com'�[39m,
      �[32m'strict-transport-security'�[39m: �[32m'max-age=31536000; includeSubdomains; preload'�[39m,
      �[32m'transfer-encoding'�[39m: �[32m'chunked'�[39m,
      vary: �[32m'Accept-Encoding, Accept, X-Requested-With'�[39m,
      �[32m'x-content-type-options'�[39m: �[32m'nosniff'�[39m,
      �[32m'x-frame-options'�[39m: �[32m'deny'�[39m,
      �[32m'x-github-api-version-selected'�[39m: �[32m'2022-11-28'�[39m,
      �[32m'x-github-media-type'�[39m: �[32m'github.v3; format=json'�[39m,
      �[32m'x-github-request-id'�[39m: �[32m'4C04:D0C2B:4268D8D:7F5CD35:670E5A06'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1728993858'�[39m,
      �[32m'x-ratelimit-resource'�[39m: �[32m'search'�[39m,
      �[32m'x-ratelimit-used'�[39m: �[32m'1'�[39m,
      �[32m'x-xss-protection'�[39m: �[32m'0'�[39m
    },
    data: {
      documentation_url: �[32m'https://docs.github.com/free-pro-team@latest/rest/overview/rate-limits-for-the-rest-api#about-secondary-rate-limits'�[39m,
      message: �[32m'You have exceeded a secondary rate limit. Please wait a few minutes before you try again. If you reach out to GitHub Support for help, please include the request ID 4C04:D0C2B:4268D8D:7F5CD35:670E5A06.'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3Aehconitin%20is%3Aclosed%20repo%3Atwentyhq%2Ftwenty&per_page=2&page=1'�[39m,
    headers: {
      accept: �[32m'application/vnd.github.v3+json'�[39m,
      �[32m'user-agent'�[39m: �[32m'octokit-rest.js/18.12.0 octokit-core.js/3.6.0 Node.js/18.20.4 (linux; x64)'�[39m,
      authorization: �[32m'token [REDACTED]'�[39m
    },
    request: { hook: �[36m[Function: bound bound register]�[39m }
  }
}
danger-results://tmp/danger-results-b6d502c1.json

Generated by 🚫 dangerJS against b97111b

greptile-apps

PR Summary

This pull request implements a compact sidebar feature as requested in issue #7143, transforming the navigation drawer into a collapsible component that displays only icons when collapsed.

Introduced isNavigationDrawerExpandedState to replace isNavigationDrawerOpenState for managing drawer expansion
Added NavigationDrawerCollapsedGreyBox component for compact view of active items
Updated NavigationDrawerHeader, NavigationDrawerItem, and other components to support both expanded and collapsed states
Implemented conditional rendering in CurrentWorkspaceMemberFavorites and NavigationDrawerSectionForObjectMetadataItems for compact view
Added navigationDrawerExpandedMemorizedState to remember drawer state across navigation

_{17 file(s) reviewed, 10 comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps · 2024-10-03T11:41:25Z

packages/twenty-front/src/modules/navigation/components/MainNavigationDrawerItems.tsx

@@ -37,6 +44,8 @@ export const MainNavigationDrawerItems = () => {
            label="Settings"
            to={'/settings/profile'}
            onClick={() => {
+              setNavigationDrawerExpandedMemorized(isNavigationDrawerExpanded);


style: Consider moving this logic to a separate function for better readability and reusability

greptile-apps · 2024-10-03T11:41:53Z

packages/twenty-front/src/modules/navigation/components/MobileNavigationBar.tsx

@@ -52,15 +49,15 @@ export const MobileNavigationBar = () => {
        if (!isCommandMenuOpened) {
          openCommandMenu();
        }
-        setIsNavigationDrawerOpen(false);
+        setIsNavigationDrawerExpanded(false);


style: Consider using 'setIsNavigationDrawerExpanded(false)' instead of 'setIsNavigationDrawerExpanded(false)' for consistency with other state updates

greptile-apps · 2024-10-03T11:42:55Z

...ont/src/modules/object-metadata/components/NavigationDrawerSectionForObjectMetadataItems.tsx

+          label={sectionTitle}
+          onClick={() => toggleNavigationSection()}
+        />
+        {isNavigationSectionOpen && renderObjectMetadataItems()}


style: Ensure renderObjectMetadataItems is only called when the section is open

greptile-apps · 2024-10-03T11:44:47Z

...-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerBackButton.tsx

+        to={navigationMemorizedUrl}
+        replace
+        onClick={() =>
+          setIsNavigationDrawerExpanded(navigationDrawerExpandedMemorized)


style: Consider adding a comment explaining the purpose of this state change

...nt/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerCollapseButton.tsx

greptile-apps · 2024-10-03T11:45:40Z

.../src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerCollapsedGreyBox.tsx

+  background-color: ${({ theme }) => theme.background.transparent.lighter};
+  border: 1px solid ${({ theme }) => theme.background.transparent.lighter};
+  border-radius: ${({ theme }) => theme.border.radius.sm};


style: Using the same color for background and border may not provide enough contrast. Consider using a slightly darker shade for the border.

greptile-apps · 2024-10-03T11:46:33Z

...twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx

+  const [isNavigationDrawerExpanded, setIsNavigationDrawerExpanded] =
+    useRecoilState(isNavigationDrawerExpandedState);
+  const isSettingsPage = useIsSettingsPage();


style: Consider memoizing this value to prevent unnecessary re-renders

greptile-apps · 2024-10-03T11:46:34Z

...twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx

-          <StyledKeyBoardShortcut className="keyboard-shortcuts">
-            {keyboard}
-          </StyledKeyBoardShortcut>
+        {isNavigationDrawerExpanded || isSettingsPage ? (


style: This conditional rendering might cause layout shifts. Consider using CSS to hide/show elements instead

...twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx

greptile-apps · 2024-10-03T11:47:48Z

...ages/twenty-front/src/modules/ui/navigation/states/navigationDrawerExpandedMemorizedState.ts

+import { atom } from 'recoil';
+import { MOBILE_VIEWPORT } from 'twenty-ui';
+
+const isMobile = window.innerWidth <= MOBILE_VIEWPORT;


logic: isMobile is set only once when the file is loaded. Consider using a hook or event listener for responsive behavior

ehconitin · 2024-10-03T11:53:18Z

@Bonapara
need clarifications on greyboxes border color and background color!
for now I have kept it according to figma theme.background.transparent.lighter for both, but that could be improved

FelixMalfait · 2024-10-03T20:31:33Z

Nice 😍

Bonapara · 2024-10-09T09:13:45Z

@ehconitin what do you mean about the clarification on the colors? We can schedule a quick call to talk about it!

A few fixes:

Make sure the border-radius are the same
The colors of the "views" icons should be tertiary to distinguish them from objects better. (including the selected view)

CleanShot.2024-10-09.at.11.07.43.mp4

The animation should be smooth like this. We shouldn't see that it's two different elements.
The gap between sections in compact mode should be smaller.

martmull · 2024-10-10T15:51:02Z

@ehconitin think we can do something like on figma with animate. We need to animate all the disapearing components in the navBar, not the navBar itself. Here is a quick proof of concept: de77463

Enregistrement.de.l.ecran.2024-10-10.a.17.47.08.mov

ehconitin · 2024-10-10T19:01:32Z

Thanks @martmull ! 🫡
Too good!

martmull · 2024-10-14T17:06:31Z

last version
https://github.com/user-attachments/assets/058a69d1-f5ad-401a-b964-0178bb0dc7ff

sonukaloshiya

[email protected]

thomtrp · 2024-10-15T09:34:00Z

...twenty-front/src/modules/ui/navigation/navigation-drawer/components/NavigationDrawerItem.tsx

+
+  width: ${(props) =>
+    !props.isNavigationDrawerExpanded
+      ? `${NAV_DRAWER_WIDTHS.menu.desktop.collapsed - 24}px`


could 24 be a shared variable with the existing menu width?

will try to

difficult tbh

greptile-apps

PR Summary

(updates since last review)

This PR continues the implementation of the compact sidebar feature, focusing on refining navigation components and improving overall functionality. Key changes include:

Introduced new hooks and components for managing the compact sidebar state and rendering
Updated various components to support both expanded and collapsed sidebar states
Improved handling of object metadata items and favorites in the navigation drawer
Refined styling and layout for better responsiveness and user experience

While the changes align with the desired behavior, there are a few points to consider:

Ensure consistent handling of loading states across components to prevent UI flickering
Review and optimize performance for components with complex rendering logic
Double-check accessibility features, especially for keyboard navigation in the compact mode

_{298 file(s) reviewed, 167 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2024-10-15T11:51:52Z

packages/twenty-front/src/modules/navigation/components/MainNavigationDrawerItems.tsx

+              setNavigationDrawerExpandedMemorized(isNavigationDrawerExpanded);
+              setIsNavigationDrawerExpanded(true);
              setNavigationMemorizedUrl(location.pathname + location.search);
            }}


logic: This logic might cause unexpected behavior if the user navigates directly to the settings page

greptile-apps · 2024-10-15T11:52:55Z

packages/twenty-front/src/modules/navigation/hooks/useIsSettingsDrawer.ts

+export const useIsSettingsDrawer = () => {
+  const isMobile = useIsMobile();
+  const isSettingsPage = useIsSettingsPage();
+  const currentMobileNavigationDrawer = useRecoilValue(
+    currentMobileNavigationDrawerState,
+  );
+  return isMobile
+    ? currentMobileNavigationDrawer === 'settings'
+    : isSettingsPage;
+};


style: Consider adding a brief comment explaining the purpose of this hook

greptile-apps · 2024-10-15T11:54:31Z

...ont/src/modules/object-metadata/components/NavigationDrawerSectionForObjectMetadataItems.tsx

+            onClick={() => toggleNavigationSection()}
+          />
+        </NavigationDrawerAnimatedCollapseWrapper>
+        {isNavigationSectionOpen && renderObjectMetadataItems()}


style: Ensure renderObjectMetadataItems is only called when the section is open

follow up - #7414

ehconitin added 3 commits October 3, 2024 14:54

changed recoil atom naming

743e786

conditional rendering changes

666fe95

fixed collapsed to settings and back width issue

b246dc1

greptile-apps bot reviewed Oct 3, 2024

View reviewed changes

ehconitin added the -PR: wip label Oct 3, 2024

ehconitin and others added 2 commits October 3, 2024 17:26

corrections

b6973a9

Merge branch 'twentyhq:main' into compactSideDrawer

162bf54

Merge branch 'twentyhq:main' into compactSideDrawer

d914d45

ehconitin marked this pull request as draft October 9, 2024 17:19

charlesBochet added the -PR: awaiting author label Oct 9, 2024

charlesBochet assigned ehconitin Oct 9, 2024

martmull mentioned this pull request Oct 10, 2024

Compact side drawer poc animation #7576

Closed

charlesBochet assigned martmull Oct 11, 2024

martmull added 8 commits October 11, 2024 10:54

Add NavigationDrawerAnimatedCollapseWrapper component

cf79d85

Use NavigationDrawerAnimatedCollapseWrapper component

76cf0a9

Remove useless usage of isNavigationDrawerExpanded

914b7ae

Use NavigationDrawerAnimatedCollapseWrapper in multiWokspaces dropdown

9b2ad85

Remove useless stuff

f380e78

Improve stuff

12cd3d1

Fix overflow issues

51911ae

Use variable

493160b

martmull force-pushed the compactSideDrawer branch from 3a2431a to 493160b Compare October 11, 2024 14:19

Factorize navigation drawer widths

7e3a9c2

martmull force-pushed the compactSideDrawer branch from 86220d9 to 7e3a9c2 Compare October 11, 2024 15:54

Add group background when collapsed

b0df7ad

martmull force-pushed the compactSideDrawer branch from c894fc9 to b0df7ad Compare October 11, 2024 17:00

martmull added 2 commits October 11, 2024 22:17

Fix breadcrumb

2f24dd6

WIP: fix stuff

90001ba

martmull force-pushed the compactSideDrawer branch from de04b42 to 7be0a3c Compare October 14, 2024 17:07

sonukaloshiya reviewed Oct 14, 2024

View reviewed changes

Fix breadcrumb

855cbcb

martmull force-pushed the compactSideDrawer branch from 7be0a3c to 855cbcb Compare October 14, 2024 17:11

martmull added 3 commits October 15, 2024 10:34

Fix opacity in settingsMobile view

dee4882

Simplify setting padding

241e232

Fix default value when collapsed

934d214

martmull force-pushed the compactSideDrawer branch from e8acb79 to 934d214 Compare October 15, 2024 09:20

thomtrp approved these changes Oct 15, 2024

View reviewed changes

Fix tests

b97111b

martmull marked this pull request as ready for review October 15, 2024 10:36

martmull merged commit a9deede into twentyhq:main Oct 15, 2024
7 of 10 checks passed

greptile-apps bot reviewed Oct 15, 2024

View reviewed changes

ehconitin mentioned this pull request Oct 16, 2024

minor fix - collapse button hover and dropdown button width #7748

Merged

bosiraphael pushed a commit that referenced this pull request Oct 16, 2024

minor fix - collapse button hover and dropdown button width (#7748)

fad0414

follow up - #7414

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat - Compact sidebar #7414

feat - Compact sidebar #7414

ehconitin commented Oct 3, 2024 •

edited by martmull

Loading

github-actions bot commented Oct 3, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Oct 3, 2024

greptile-apps bot Oct 3, 2024

greptile-apps bot Oct 3, 2024

greptile-apps bot Oct 3, 2024

greptile-apps bot Oct 3, 2024

greptile-apps bot Oct 3, 2024

greptile-apps bot Oct 3, 2024

greptile-apps bot Oct 3, 2024

ehconitin commented Oct 3, 2024

FelixMalfait commented Oct 3, 2024

Bonapara commented Oct 9, 2024

martmull commented Oct 10, 2024

ehconitin commented Oct 10, 2024

martmull commented Oct 14, 2024

sonukaloshiya left a comment

thomtrp Oct 15, 2024

martmull Oct 15, 2024

martmull Oct 15, 2024

greptile-apps bot left a comment

greptile-apps bot Oct 15, 2024

greptile-apps bot Oct 15, 2024

greptile-apps bot Oct 15, 2024

feat - Compact sidebar #7414

feat - Compact sidebar #7414

Conversation

ehconitin commented Oct 3, 2024 • edited by martmull Loading

To Do

github-actions bot commented Oct 3, 2024 • edited Loading

Log

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Oct 3, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 3, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 3, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 3, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 3, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 3, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 3, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 3, 2024

Choose a reason for hiding this comment

ehconitin commented Oct 3, 2024

FelixMalfait commented Oct 3, 2024

Bonapara commented Oct 9, 2024

martmull commented Oct 10, 2024

ehconitin commented Oct 10, 2024

martmull commented Oct 14, 2024

sonukaloshiya left a comment

Choose a reason for hiding this comment

thomtrp Oct 15, 2024

Choose a reason for hiding this comment

martmull Oct 15, 2024

Choose a reason for hiding this comment

martmull Oct 15, 2024

Choose a reason for hiding this comment

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Oct 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 15, 2024

Choose a reason for hiding this comment

greptile-apps bot Oct 15, 2024

Choose a reason for hiding this comment

ehconitin commented Oct 3, 2024 •

edited by martmull

Loading

github-actions bot commented Oct 3, 2024 •

edited

Loading