search

facebook / lexical

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

Bug: Changing the font size when inputting does not take effect #6000

Open zhuba-Ahhh opened 2 weeks ago

zhuba-Ahhh commented 2 weeks ago

Lexical version: latest

Steps To Reproduce

  1. open https://playground.lexical.dev/
  2. Change font size when inputting

https://github.com/facebook/lexical/assets/84793349/5d13a4a3-f4f7-44fa-833a-3416708fa2db

Link to code example:

The current behavior

Changing the font size when inputting does not take effect

The expected behavior

Input can change font size

CARPEDIEM05 commented 2 weeks ago

Hi @StyleT , I want to try this. This is the first time I am contributing to open-source. How can I assign this to myself?

CARPEDIEM05 commented 2 weeks ago

Hi @zhuba-Ahhh @StyleT, I somewhat got the solution. It is taking effect but we have to press enter as it is been stated in onKeyDown that you have to press enter to change the state of the font size.

zhuba-Ahhh commented 2 weeks ago

Hi @zhuba-Ahhh @StyleT, I somewhat got the solution. It is taking effect but we have to press enter as it is been stated in onKeyDown that you have to press enter to change the state of the font size.

Yes, I have found that pressing Enter is effective, but simply typing to change the value in the input box is not effective. You can try

CARPEDIEM05 commented 2 weeks ago

Hi @zhuba-Ahhh As like we are handling the onChange and onKeyDown event, we have to handle, onBlur event. My suggestion is to remove onKeyDown handler and just use the onBlur event to handle the change of the font.

zhuba-Ahhh commented 2 weeks ago

Hi @zhuba-Ahhh As like we are handling the onChange and onKeyDown event, we have to handle, onBlur event. My suggestion is to remove onKeyDown handler and just use the onBlur event to handle the change of the font.

Yes, you can try it like this?

image
CARPEDIEM05 commented 2 weeks ago

Hi @zhuba-Ahhh , no this won't work as this will violate the max and min allowed size of the font.

zhuba-Ahhh commented 2 weeks ago

I still think deleting the onKeyDown event is not a good idea

CARPEDIEM05 commented 2 weeks ago

You might be right, who will be the right person to comment on this? You or someone else? I am new to this so I don't know.

CARPEDIEM05 commented 2 weeks ago

According to me

  1. Either to remove the onKeyDown event and add the onBlur event
  2. We can include both the event

What do you think?

ebads67 commented 2 weeks ago

I'm not sure if this is an issue. To change the font size you need to select the text that you need to change the font size for. If there is no selection, then it changes the font only for the text you will enter after font change.

StyleT commented 2 weeks ago

Hi @StyleT , I want to try this. This is the first time I am contributing to open-source. How can I assign this to myself?

Just comment here that you'll work on it and submit a PR ;) Even if 2+ people submit concurrent PRs (never happens, almost) - it's ok.

CARPEDIEM05 commented 2 weeks ago

Hey @StyleT , I am working on it, i have submitted my code for review. So hope so that it will be assign to me.