twentyhq / twenty

Building a modern alternative to Salesforce, powered by the community.
https://twenty.com
GNU Affero General Public License v3.0
15.54k stars 1.63k forks source link

Fix post merge Revamp navigation bar #6285

Closed lucasbordeau closed 1 month ago

lucasbordeau commented 1 month ago

Scope & Context

After merge of https://github.com/twentyhq/twenty/pull/6031, we have a lot of small fixes and refactors to do. We left them aside to release a working feature.

Technical inputs

greptile-apps[bot] commented 1 month ago

Implementation Steps

  1. Fix RecordShowPage story: Update the MSW mocks in RecordShowPage.stories.tsx to match the new request structure.

  2. Merge useScrollToPosition and useScrollRestoration: Combine logic from useScrollToPosition and useScrollRestoration in /src/hooks/ into a single hook, ensuring all scroll restoration functionality is preserved.

  3. Rename RecordIndexRecordChip to RecordIdentifierChip: Rename the component in /src/modules/object-record/record-index/components/RecordIndexRecordChip.tsx to RecordIdentifierChip.tsx and update all imports.

  4. Rename isFieldChipDisplay to isFieldIdentifierDisplay: Rename the variable in /src/modules/object-record/record-field/meta-types/display/components/RelationFromManyFieldDisplay.tsx and update all references.

  5. Refactor RecordIndexContainer: Move logic from /src/modules/object-record/record-index/components/RecordIndexContainer.tsx to a separate hook or utility file. Use useGeCurrent for fetching current state.

  6. Use recoil state instead of useFindRecordCursorFromFindManyCacheRootQuery: Replace useFindRecordCursorFromFindManyCacheRootQuery with a recoil state in /src/modules/object-record/record-index/components/RecordIndexContainer.tsx.

  7. Refactor useRecordIdsFromFindManyCacheRootQuery: Modify useRecordIdsFromFindManyCacheRootQuery in /src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts to only execute if a cursor exists in the recoil state.

  8. Refactor findView and view: any: Update findView function and view type in /src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts to use a more specific type.

  9. Refactor useRecordShowPagePagination: Generalize useRecordShowPagePagination in /src/modules/object-record/record-show/hooks/useRecordShowPagePagination.ts to work with different views.

  10. Refactor ROW_HEIGHT extract: Extract ROW_HEIGHT constant to a shared constants file and update all references.

  11. Refactor URL building: Move URL building logic to a shared utility file and update all instances where URLs are constructed.

References

/packages/twenty-front/src/pages/object-record/RecordShowPage.tsx /packages/twenty-front/src/hooks/useScrollRestoration.ts /packages/twenty-front/src/modules/object-record/record-index/components/RecordIndexContainer.tsx /packages/twenty-front/src/modules/object-record/record-show/hooks/useRecordShowPage.ts /packages/twenty-front/src/modules/object-record/record-show/graphql/operations/factories/findOneRecordForShowPageOperationSignatureFactory.ts /packages/twenty-front/src/modules/object-record/record-show/components/RecordShowContainer.tsx /packages/twenty-front/src/modules/object-record/record-show /packages/twenty-front/src/modules/object-record/record-index/components /packages/twenty-front/src/modules/object-record/record-show/hooks /packages/twenty-front/src/modules/object-record/record-show/record-detail-section/components /packages/twenty-front/src/modules/object-record/record-right-drawer/components /packages/twenty-front/src/modules/object-record/record-field/meta-types/display/components/RelationFromManyFieldDisplay.tsx

#### About Greptile This response provides a starting point for your research, not a precise solution. Help us improve! Please leave a ๐Ÿ‘ if this is helpful and ๐Ÿ‘Ž if it is irrelevant. [Ask Greptile](https://app.greptile.com/chat/github/twentyhq/twenty/main) ยท [Edit Issue Bot Settings](https://app.greptile.com/apps/github)
lucasbordeau commented 1 month ago

@FelixMalfait @charlesBochet

Passing the cursor from table seems useless, because we can already have it from the return of the findOne request by using the same query variables than the passed view.

FelixMalfait commented 1 month ago

The assumption that led to passing the parameter was that recomputing the position / converting the sorts into filters would be too expensive, but great if it's not!