twentyhq / twenty

Building a modern alternative to Salesforce, powered by the community.
https://twenty.com
Other
23.8k stars 2.46k forks source link

Refactor RecordInlineCell tree with a Context to avoid props drilling #6335

Closed lucasbordeau closed 3 months ago

lucasbordeau commented 4 months ago

Scope & Context

All the hierarchy of RecordInlineCell components

Technical inputs

We currently use a lot of props drilling, with components slicing props types of more than 10 properties, it's difficult to maintain and should be better placed in a context at the top of the hierarchy since those values are all readonly and defined when the component is mounted.

Example in RecordInlineCellValue :

type RecordInlineCellValueProps = Pick<
  RecordInlineCellContainerProps,
  | 'displayModeContent'
  | 'customEditHotkeyScope'
  | 'editModeContent'
  | 'editModeContentOnly'
  | 'isDisplayModeFixHeight'
  | 'disableHoverEffect'
  | 'readonly'
  | 'buttonIcon'
  | 'loading'
  | 'showLabel'
  | 'label'
  | 'isCentered'
>;

We shouldn't have even one props because everything is passed at the top of the hierarchy.

Please note that it shouldn't be put in FieldContext as it is not in the domain of a Field but of RecordInlineCell which are two different abstractions.

hansol-y commented 4 months ago

Hi @lucasbordeau, can you please assign me for this issue? I'm interested in this one!

Edit: I worked on this issue and made PR6359