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

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

Closed
bedre7 opened this issue Nov 11, 2024 · 5 comments · Fixed by #6845
Closed

Comments

@bedre7
Copy link
Contributor

bedre7 commented Nov 11, 2024

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

Autocomplete.format.bug.mp4

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
Copy link
Contributor Author

bedre7 commented Nov 11, 2024

I would like 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
Copy link
Collaborator

etrepum commented Nov 11, 2024

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
Copy link
Contributor Author

bedre7 commented Nov 12, 2024

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

@bedre7
Copy link
Contributor Author

bedre7 commented Nov 12, 2024

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
Copy link
Collaborator

etrepum commented Nov 12, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants