Activity Editor hot key scope management

[Muralidhar22]

This pull request closes #3197

Changes

1. Refactored activity body editor as middle container in to make body editor container take up the entire space between activity editor upper and activity comment.
2. Added useListenClickOutside to the ActivityBodyEditor to focus on last empty line.
3. Added Esc hot key listener for activity title, activity comment , activity targets.
4. Added listener for text key input to focus on activity body editor when it's right drawer scope.

Checklist

• Focus on the last empty line if we click anywhere below the last line, in the activity editor body, If there's no last empty line, create a new empty line in the last position.
• Listen for keyboard events, and if the user types something, put the focus in the last empty line and write in what the user typed.
• Exit the focus on the activity editor body then close the right drawer if there's no focus on the activity editor body or on the fields in the header.

[charlesBochet]

@Muralidhar22 is this ready for review? :)

[Muralidhar22]

Hi @charlesBochet below two things am not able figure out

Listen for keyboard events, and if the user types something, put the focus in the last empty line and write in what the user typed.

• For example how to check if cursor is focused on activity title/ comment or any fields are open and close them and start entering text on body editor

Exit the focus on the activity editor body then close the right drawer if there's no focus on the activity editor body or on the fields in the header.

• To listen to title, comment input which one is focused and blur that respective field.
• Fixing Esc hot key scope for dueAt and assignee fields

[lucasbordeau]

Hi @Muralidhar22, I think that for both of your questions, you would need some kind of recoil state to know if you're editing something inside the right drawer ? And put it at true/false at specific places ?

I don't know if it's better to use a global boolean recoil state or a state that stores what we are currently editing.

Maybe each component should be responsible for its own 'esc' listener and provide an onBlur event so you can connect it to the logic of modifying your state.

[Muralidhar22]

Hey @lucasbordeau I'll try and see what I can do and update here

[Muralidhar22]

@lucasbordeau

• Listen for keyboard events, and if the user types something, put the focus in the last empty line and write in what the user typed.

• Exit the focus on the activity editor body then close the right drawer if there's no focus on the activity editor body or on the fields in the header.

Was able to add the behaviour required in above two points without recoil state,

• By having separate esc handlers for activity editor fields.
• Focusing on editor body when user types only when it's in the scope of right drawer scope

[github-actions]

TODOs/FIXMEs:

• // TODO: remove: packages/twenty-front/src/modules/activities/components/ActivityEditor.tsx

Generated by 🚫 dangerJS against 1836bf6

[lucasbordeau]

For RecordInlineCell :

• We want to allow someone to listen to open and close, which is more relevant to how Cells and InlineCells work from the outside, for this we need to rely on FieldContext, and you can modify GenericFieldContextType for this.
• Let's replace onBlur by onClose and onFocus by onOpen on this context and provide them to the field contexts we use to wrap our RecordInlineCell in the different activity children components.

So instead of having this :

export const RecordInlineCell = ({
  onBlur,
  onFocus,
}: RecordInlineCellProps) => {
  const { fieldDefinition, entityId } = useContext(FieldContext);

We'll have this :

export const RecordInlineCell = () => {
  const { fieldDefinition, entityId, onOpen, onClose } = useContext(FieldContext);

Then for example in ActivityTargetsInlineCell :

  const { FieldContextProvider } = useFieldContext({
    objectNameSingular: CoreObjectNameSingular.Activity,
    objectRecordId: activity?.id ?? '',
    fieldMetadataName: 'activityTargets',
    fieldPosition: 2,
    onOpen: () => {
      setYourRecoilState(...)
    }
  });

You'll have to modify the useFieldContext hook.

With all of this you should have a solid understanding of how we manage generic fields, cells and inline cells.

Good luck 🫡

[Muralidhar22]

@lucasbordeau thanks for your inputs, will try to make the changes and update here.

[charlesBochet]

@Muralidhar22 @lucasbordeau I've merged main into the branch, fixed broken tests and fixed the logic to append to the drawer content (which was not working in the case of non-text block)

I think we might be able to simplify even a bit more, I'll need to test a bit more deeply

[charlesBochet]

this change seems unrelated to the PR

[charlesBochet]

this is not useful anymore. It also create a discrepancy with RecordTableCell

[charlesBochet]

ActivityTargetsInlineCell should behave as RecordInlineCell, conceptually it's the same thing

[charlesBochet]

this is actually not needed, we should leave useInlineCell handle it (there was a bug as is too low in the component hiearchy making this useInlineCell out of context)

[charlesBochet]

not needed, already handled by useInlineCell

[charlesBochet]

@Muralidhar22 I have completed the review and simplified a few things. Feel free to take a look at the changes! In the idea, we should not introduce too much logic. I think the InlineCell logic is still a bit hard to understand and twenty team should keep improving the developer experience so it's easier for external contributors to contribute!

[charlesBochet]

forgot to use it!

[charlesBochet]

I don't think we should add this MiddleContainer

Changes have been made!

[charlesBochet]

@Muralidhar22 Thanks a lot for the big part of the work, good to go!