Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Timebox] Refactor RecordShow components hierarchy #378

Open
10 tasks
lucasbordeau opened this issue Jan 31, 2025 · 0 comments
Open
10 tasks

[Timebox] Refactor RecordShow components hierarchy #378

lucasbordeau opened this issue Jan 31, 2025 · 0 comments
Assignees

Comments

@lucasbordeau
Copy link

lucasbordeau commented Jan 31, 2025

Scope & Context

ActivityTargetsInlinceCell has a hard re-render caused by the RecordShowContainer.

This is caused by subscriptions made at all levels with grouping hooks which is a pattern we try to avoid.

This is a high priority refactor because right now anything that is inside a RecordShowContainer will have a hard re-render for any field change.

This refactor would also help for the next iteration with modular show container could be hard to make with the current state.

Technical inputs

Here is a break-down of this refactor :

  • Refactor RecordShowPage and useRecordShowPage hook
    • Create ObjectRecordContext ⇒ name + id OR try re-using an existing context that might fit
  • Refactor RecordShowContainer and useRecordShowData
  • Refactor ShowPageSubContainer ⇒ remove object-record from ui/layout
    • Rename to RecordShowPageSubContainer
  • Refactor CardComponent should take its object record from context
  • Refactor FieldsCard
  • Refactor ActivityTargetsInlineCell
    • Get activity from record store + object record context
    • Remove useEffects and set directly selected records in

All the refactor should also take care of the following :

  • Removing any props that can be replaced by a call to useContext (most probably 100% can be replaced in this refactor, props drill-down should be avoided and here it is most of the time just object name and record id)
  • Remove any "grouping" hook : split in multiple unit hooks, call states directly where they are needed, remove unnecessary useEffect state synchronization logic, move effects to Effect components
@charlesBochet charlesBochet changed the title Refactor RecordShow components hierarchy [Timebox] Refactor RecordShow components hierarchy Feb 3, 2025
@FelixMalfait FelixMalfait transferred this issue from twentyhq/twenty Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🔖 Planned
Development

No branches or pull requests

3 participants