feat: Revamp navigation bar #6031

AdityaPimpalkar · 2024-06-26T08:29:31Z

closes: #4428

Testing for fetchMoreRecords is pending, along with component tests

greptile-apps

PR Summary

Introduced useRecordShowPagePagination hook for record show page pagination
Added viewBarId prop to RecordTable and RecordTableWithWrappers
Updated RecordTableContextProps to include viewBarId
Enhanced PageHeader with pagination buttons and updated styles
Modified useChipFieldDisplay to append view ID to linkToShowPage

_{7 file(s) reviewed, 2 comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps · 2024-06-26T08:30:04Z

...ages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts

+  const navigateToPreviousRecord = () => {
+    const prevRecord = objectRecords[currentRecordPosition - 1];
+    navigate(
+      `/object/${objectNameSingular}/${prevRecord.id}${
+        viewIdQueryParam ? `?view=${viewIdQueryParam}` : ''
+      }`,
+    );
+    setCurrentRecordPosition(currentRecordPosition - 1);
+  };


🧠 logic: Ensure currentRecordPosition - 1 is within bounds before accessing objectRecords.

greptile-apps · 2024-06-26T08:30:04Z

...ages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts

+  const navigateToNextRecord = () => {
+    const nextRecord = objectRecords[currentRecordPosition + 1];
+    navigate(
+      `/object/${objectNameSingular}/${nextRecord.id}${
+        viewIdQueryParam ? `?view=${viewIdQueryParam}` : ''
+      }`,
+    );
+    setCurrentRecordPosition(currentRecordPosition + 1);
+  };


🧠 logic: Ensure currentRecordPosition + 1 is within bounds before accessing objectRecords.

greptile-apps

PR Summary

(updates since last review)

Enhanced pagination logic in useRecordShowPagePagination.ts
Added viewBarId property to context in RecordTableCell.perf.stories.tsx
Replaced back button with close button in PageHeader.tsx
Introduced pagination and close button in RecordShowPage.tsx

_{4 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

Reordered imports in useRecordShowPagePagination.ts
Reformatted code for better readability
Ensured correct invocation of fetchMoreRecords
Adjusted logic for fetching more records for clarity

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

Updated mock data for peopleMock
Added new GraphQL query handler for FindManyPeople
Improved accuracy of Storybook tests for RecordShowPage component

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

greptile-apps

PR Summary

(updates since last review)

Simplified useEffect dependency array in useRecordShowPagePagination.ts
Potential impact on pagination state management and updates

_{1 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

FelixMalfait · 2024-06-27T08:51:19Z

Pass view id to showpage on Kanban

greptile-apps

PR Summary

(updates since last review)

Added setRecordPosition function to ChipFieldDisplay.tsx for handling record positioning
Introduced setRecordPosition in useChipFieldDisplay.ts using Recoil for state management
Added recordPositionInternalState.ts to define state for tracking record positions
Updated useRecordShowPagePagination.ts to use Recoil for record position state management
Enhanced EntityChip.tsx with an optional onClick prop for custom click handlers

_{5 file(s) reviewed, no comment(s)}
_{Edit PR Review Bot Settings}

FelixMalfait

I made a lot of small comments but overall it is great! Thanks!!!

...ages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts

.../twenty-front/src/modules/object-record/record-field/meta-types/hooks/useChipFieldDisplay.ts

packages/twenty-front/src/pages/object-record/RecordShowPage.tsx

...ages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts

github-actions · 2024-07-15T12:14:36Z

	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 6CC7:13ED99:67554AF:BF90216:669669F4.
    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%3AAdityaPimpalkar%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, 16 Jul 2024 12:39:16 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'6CC7:13ED99:67554AF:BF90216:669669F4'�[39m,
      �[32m'x-ratelimit-limit'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-remaining'�[39m: �[32m'30'�[39m,
      �[32m'x-ratelimit-reset'�[39m: �[32m'1721133616'�[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 6CC7:13ED99:67554AF:BF90216:669669F4.'�[39m
    }
  },
  request: {
    method: �[32m'GET'�[39m,
    url: �[32m'https://api.github.com/search/issues?q=is%3Apr%20author%3AAdityaPimpalkar%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-bd6f7ad9.json

Generated by 🚫 dangerJS against b03dbf2

lucasbordeau · 2024-07-15T14:11:19Z

packages/twenty-front/src/modules/object-record/hooks/useFetchAllRecordIds.ts

@@ -37,7 +37,7 @@ export const useFetchAllRecordIds = <T>({
    const firstQueryResult =
      findManyRecordsDataResult?.data?.[objectMetadataItem.namePlural];

-    const totalCount = firstQueryResult?.totalCount ?? 1;
+    const totalCount = firstQueryResult?.totalCount ?? 0;


...es/twenty-front/src/modules/object-record/record-field/states/recordPositionInternalState.ts

FelixMalfait · 2024-07-15T14:27:42Z

...ages/twenty-front/src/modules/object-record/record-index/components/RecordIndexContainer.tsx

+          )
+          .getValue();
+
+        // TODO: use URL builder


https://reactrouter.com/en/main/utils/generate-path

FelixMalfait · 2024-07-15T14:28:18Z

packages/twenty-front/src/modules/object-record/record-show/hooks/useReasd.ts

@@ -0,0 +1,30 @@
+import { RecordGqlRefEdge } from '@/object-record/cache/types/RecordGqlRefEdge';


Strange file name?

FelixMalfait · 2024-07-15T14:30:15Z

...rc/modules/object-record/record-table/record-table-body/components/RecordTableBodyEffect.tsx

+          (record) => record.id === lastShowPageRecordId,
+        );
+
+        const positionInPx = recordPosition * 32;


use row height?

FelixMalfait · 2024-07-15T14:34:27Z

packages/twenty-ui/src/utilities/isDefined.ts

@@ -0,0 +1,4 @@
+import { isNull, isUndefined } from '@sniptt/guards';
+
+export const isDefined = <T>(value: T | null | undefined): value is T =>


should we have it in both places? maybe just in twenty-ui then?

charlesBochet · 2024-07-16T11:54:42Z

packages/twenty-front/src/hooks/useScrollToPosition.ts

@@ -0,0 +1,20 @@
+import { overlayScrollbarsState } from '@/ui/utilities/scroll/states/overlayScrollbarsState';


this looks very similar to useScrollRestoration? could we merge both?

charlesBochet · 2024-07-16T12:00:16Z

packages/twenty-front/src/modules/activities/calendar/components/CalendarEventRow.tsx

@@ -169,7 +169,9 @@ export const CalendarEventRow = ({
                  ? `${participant.firstName} ${participant.lastName}`
                  : participant.displayName
              }
-              entityId={participant.workspaceMemberId ?? participant.personId}
+              placeholderColorSeed={


charlesBochet · 2024-07-16T12:06:32Z

packages/twenty-front/src/modules/object-metadata/utils/getLabelIdentifierFieldValue.ts

@@ -17,7 +17,7 @@ export const getLabelIdentifierFieldValue = (
  }

  if (isDefined(labelIdentifierFieldMetadataItem?.name)) {
-    return record[labelIdentifierFieldMetadataItem.name] as string | number;
+    return String(record[labelIdentifierFieldMetadataItem.name]);


why is this needed?

charlesBochet · 2024-07-16T12:12:23Z

...nt/src/modules/object-record/record-field/meta-types/display/components/ChipFieldDisplay.tsx

-      avatarUrl={recordChipData.avatarUrl ?? ''}
-      linkToEntity={recordChipData.linkToShowPage}
+  return isLabelIdentifier ? (
+    <RecordIndexRecordChip


Not sure about this naming RecordIndexRecordChip. should be RecordIdentifierChip?

charlesBochet · 2024-07-16T12:14:16Z

...-front/src/modules/object-record/record-field/meta-types/display/utils/isFieldChipDisplay.ts

+import { isFieldNumber } from '@/object-record/record-field/types/guards/isFieldNumber';
+import { isFieldText } from '@/object-record/record-field/types/guards/isFieldText';
+
+export const isFieldChipDisplay = (


I would rename this to isFieldIdentifierDisplay

charlesBochet · 2024-07-16T12:21:06Z

...-front/src/modules/object-record/record-show/hooks/useRecordIdsFromFindManyCacheRootQuery.ts

+import { useApolloClient } from '@apollo/client';
+import { createApolloStoreFieldName } from '~/utils/createApolloStoreFieldName';
+
+export const useRecordIdsFromFindManyCacheRootQuery = ({


regarding the comment above, here it would be find if fieldVariables was also containing the cursor

charlesBochet · 2024-07-16T12:21:50Z

...ages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts

+}) => {
+  if (!viewId) {
+    return views.find(
+      (view: any) =>


any could be typed here

charlesBochet · 2024-07-16T12:23:41Z

...ages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts

+      `/object/${objectNameSingular}/${recordBefore.id}${
+        viewIdQueryParam ? `?view=${viewIdQueryParam}` : ''
+      }`,
+      {


do we use it?

charlesBochet · 2024-07-16T12:29:33Z

...ages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts

+  }
+};
+
+export const useRecordShowPagePagination = (


I feel this could be in a separate module "navigate-in-view" or something like this and could be re-used in other places later

charlesBochet · 2024-07-16T12:30:24Z

...twenty-front/src/modules/object-record/record-table/components/RecordTableInternalEffect.tsx

@@ -33,7 +33,7 @@ export const RecordTableInternalEffect = ({
    callback: () => {
      leaveTableFocus();
    },
-    mode: ClickOutsideMode.comparePixels,
+    mode: ClickOutsideMode.compareHTMLRef,


why this change? don't we have an issue with portals? (filter dropdown, field input) ?

I changed it in a recent PR but this introduced the bug of double click to open

AdityaPimpalkar added 3 commits June 26, 2024 09:19

pass viewBarId to RecordTableContext

096582a

useRecordShowPagePagination hook

64a0bc3

add hook functions to page header

6896aa3

greptile-apps bot reviewed Jun 26, 2024

View reviewed changes

AdityaPimpalkar added 4 commits June 26, 2024 16:24

add fetchMoreRecords functionality to pagination

c3bedd5

add viewBarId to RecordTableCell perf story

abd3bc3

add onClosePage prop to RecordShowPage

0fc0add

refacto hook

9afe013

greptile-apps bot reviewed Jun 26, 2024

View reviewed changes

lint fix

2be260b

greptile-apps bot reviewed Jun 26, 2024

View reviewed changes

AdityaPimpalkar added 2 commits June 27, 2024 00:20

storybook fix

53c8aeb

deps fix

9538762

greptile-apps bot reviewed Jun 26, 2024

View reviewed changes

charlesBochet added the -PR: awaiting author label Jun 30, 2024

add recordPositionInternalState

76acab8

greptile-apps bot reviewed Jul 4, 2024

View reviewed changes

FelixMalfait reviewed Jul 5, 2024

View reviewed changes

lucasbordeau added 5 commits July 11, 2024 11:15

Merge branch 'main' into revamp-navigation-bar

a347da3

Fix

4aa0b91

Refactored Avatar and RecordChip

cd14333

Fix

fb1df94

Fixed errors

2f0913f

FelixMalfait assigned lucasbordeau Jul 13, 2024

lucasbordeau added 2 commits July 15, 2024 11:48

WIP

091c5a6

Merge branch 'main' into revamp-navigation-bar

462f9f6

Fixes from PR

2e2b979

Finnished

e1d0a66

lucasbordeau reviewed Jul 15, 2024

View reviewed changes

lucasbordeau added 2 commits July 15, 2024 16:12

Fix

d411037

Fix

a1ea522

FelixMalfait reviewed Jul 15, 2024

View reviewed changes

...es/twenty-front/src/modules/object-record/record-field/states/recordPositionInternalState.ts Show resolved Hide resolved

FelixMalfait reviewed Jul 15, 2024

View reviewed changes

lucasbordeau added 5 commits July 15, 2024 18:14

WIP

f0a2ff3

Used query before and after cursor instead of fetchMore loops

e383f9b

Fixes

861d032

Merge branch 'main' into revamp-navigation-bar

e292d8e

Fix test

b03dbf2

charlesBochet reviewed Jul 16, 2024

View reviewed changes

ijreilly enabled auto-merge (squash) July 16, 2024 12:37

ijreilly approved these changes Jul 16, 2024

View reviewed changes

ijreilly merged commit 4a67cfa into twentyhq:main Jul 16, 2024
6 of 11 checks passed

lucasbordeau mentioned this pull request Jul 16, 2024

Fix post merge Revamp navigation bar #6285

Closed

11 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Revamp navigation bar #6031

feat: Revamp navigation bar #6031

AdityaPimpalkar commented Jun 26, 2024 •

edited

Loading

greptile-apps bot left a comment

greptile-apps bot Jun 26, 2024

greptile-apps bot Jun 26, 2024

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot left a comment

greptile-apps bot left a comment

FelixMalfait commented Jun 27, 2024

greptile-apps bot left a comment

FelixMalfait left a comment

github-actions bot commented Jul 15, 2024 •

edited

Loading

lucasbordeau Jul 15, 2024

FelixMalfait Jul 15, 2024

FelixMalfait Jul 15, 2024

FelixMalfait Jul 15, 2024

FelixMalfait Jul 15, 2024

charlesBochet Jul 16, 2024

charlesBochet Jul 16, 2024

charlesBochet Jul 16, 2024

charlesBochet Jul 16, 2024

charlesBochet Jul 16, 2024

charlesBochet Jul 16, 2024

charlesBochet Jul 16, 2024

charlesBochet Jul 16, 2024

charlesBochet Jul 16, 2024

charlesBochet Jul 16, 2024

lucasbordeau Jul 16, 2024

		@@ -0,0 +1,30 @@
		import { RecordGqlRefEdge } from '@/object-record/cache/types/RecordGqlRefEdge';

		@@ -0,0 +1,4 @@
		import { isNull, isUndefined } from '@sniptt/guards';

		export const isDefined = <T>(value: T \| null \| undefined): value is T =>

		@@ -0,0 +1,20 @@
		import { overlayScrollbarsState } from '@/ui/utilities/scroll/states/overlayScrollbarsState';

feat: Revamp navigation bar #6031

feat: Revamp navigation bar #6031

Conversation

AdityaPimpalkar commented Jun 26, 2024 • edited Loading

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot Jun 26, 2024

Choose a reason for hiding this comment

greptile-apps bot Jun 26, 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 left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

FelixMalfait commented Jun 27, 2024

greptile-apps bot left a comment

Choose a reason for hiding this comment

PR Summary

FelixMalfait left a comment

Choose a reason for hiding this comment

github-actions bot commented Jul 15, 2024 • edited Loading

Log

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AdityaPimpalkar commented Jun 26, 2024 •

edited

Loading

github-actions bot commented Jul 15, 2024 •

edited

Loading