twenty icon indicating copy to clipboard operation
twenty copied to clipboard

on click focus on activity body editor

Open Muralidhar22 opened this issue 1 year ago • 5 comments

This pull request closes https://github.com/twentyhq/twenty/issues/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

  • [x] 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.
  • [x] 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.
  • [x] 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.

Muralidhar22 avatar Jan 22 '24 03:01 Muralidhar22

@Muralidhar22 is this ready for review? :)

charlesBochet avatar Jan 23 '24 14:01 charlesBochet

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

Muralidhar22 avatar Jan 24 '24 04:01 Muralidhar22

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.

lucasbordeau avatar Jan 24 '24 17:01 lucasbordeau

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

Muralidhar22 avatar Jan 26 '24 07:01 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

Muralidhar22 avatar Jan 29 '24 20:01 Muralidhar22

TODOs/FIXMEs:

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

Generated by :no_entry_sign: dangerJS against 1836bf6ec6cbe06729d0a9e77658dae68db1624c

github-actions[bot] avatar Feb 09 '24 15:02 github-actions[bot]

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 🫡

lucasbordeau avatar Feb 09 '24 15:02 lucasbordeau

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

Muralidhar22 avatar Feb 09 '24 16:02 Muralidhar22

@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 avatar Feb 12 '24 17:02 charlesBochet

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

charlesBochet avatar Feb 13 '24 20:02 charlesBochet