search

facebook / lexical

Lexical is an extensible text editor framework that provides excellent reliability, accessibility and performance.
https://lexical.dev
MIT License
19.72k stars 1.68k forks source link

Bug: AutocompletePlugin doesn't suggest and autocomplete in the same format as the prefix #6816

Open bedre7 opened 3 days ago

bedre7 commented 3 days ago

Lexical version: v0.20.0

Steps To Reproduce

  1. Go to lexical playground
  2. Turn on Autocomplete from the settings
  3. Change the text format to bold, italic...
  4. Type in some common word
  5. Compare the text format of the suggested word and the word you typed
  6. Press TAB

The current behavior

https://github.com/user-attachments/assets/070fa5a5-8c3c-4e78-9c16-79a1233e19a0

The expected behavior

  1. Suggest in the same format as the prefix
  2. Complete the word in the same format as the prefix when TAB is pressed
bedre7 commented 3 days ago

I want to work on this issue. I have a WIP. Fixing the format after pressing TAB is straightforward, but fixing the format for the suggestion requires making the AutocompleteNode extend TextNode instead of DecoratorNode. This is because DecoratorNode does not have a format property. I extended TextNode instead, and it worked, but I'm not sure if it's worth fixing the format for the suggestion as long as the format is fixed for the text after pressing TAB. Either way is fine with me. I need your opinion on this.

etrepum commented 3 days ago

This is just a demonstration node, not something that is offered for direct re-use, so I don't think that any of us have very strong opinions about how exactly it should work. I can think of plenty of ways that might make sense for getting the decorator to display the format, or not using a decorator at all (e.g. adding a class and data attribute with the suggestion text to the TextNode's DOM temporarily and using a pseudo-element to display the suggestion), you could even just cloneNode on the TextNode's DOM, replace the text, and add a class to the container to override the color or add opacity or something like that.

I would start with fixing the insertion (probably just using selection.insertText instead of the replaceNode) and then maybe in a separate pass worry about the display of the autocomplete or any refactoring of how it works.

bedre7 commented 2 days ago

Thanks for the suggestions, I've made a note of that.

bedre7 commented 2 days ago

This is just a demonstration node, not something that is offered for direct re-use, so I don't think that any of us have very strong opinions about how exactly it should work. I can think of plenty of ways that might make sense for getting the decorator to display the format, or not using a decorator at all (e.g. adding a class and data attribute with the suggestion text to the TextNode's DOM temporarily and using a pseudo-element to display the suggestion), you could even just cloneNode on the TextNode's DOM, replace the text, and add a class to the container to override the color or add opacity or something like that.

I would start with fixing the insertion (probably just using selection.insertText instead of the replaceNode) and then maybe in a separate pass worry about the display of the autocomplete or any refactoring of how it works.

Since I’m moving the suggested text inside the node, I don’t think it’s necessary to call setSuggestion, which comes from useSharedAutocompleteContext. I was thinking about removing both useSharedAutocompleteContext and AutocompleteComponent, but I couldn't quite understand what this context is used for, particularly concerning the listeners set and the publish-subscribe logic. Can you please clarify that?

etrepum commented 2 days ago

Sorry, I'm not familiar with this particular playground component to answer this without doing a full audit of what's going on. Just try it out, if it doesn't work the way you think it does then it should be clear when testing the component. Since this is just a demo component that you have to copy+modify to use it in your own projects, the risk is fairly low.