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: fix code block lines #4361 #4375

Closed rajarshisamaddar closed 11 months ago

rajarshisamaddar commented 1 year ago

The Code block, which previously did not display line numbers until the very end, has been corrected in this pull request.

Closes #4361 Bug: code block lines

Testing instructions:

  1. Install all the required packages from the package.json file.
  2. Launch the Lexical Playground and evaluate the code block that now features horizontal scrolling with proper line numbering.

Additional notes:

FB Lexical

vercel[bot] commented 1 year ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lexical ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 21, 2023 11:15am
lexical-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 21, 2023 11:15am
affvalente commented 1 year ago

Empty lines seems fixed, but now text does not wrap

image

rajarshisamaddar commented 1 year ago

Empty lines seems fixed, but now text does not wrap

image

Okay, I'll fix that

rajarshisamaddar commented 1 year ago

@affvalente I decided to address this matter but later discovered that the majority of code or text editors do not permit users to move to the next line. Since this is a code block, the text appears as comments that cannot be split into multiple lines unless it is a multi-line comment.

To sum up, I believe the functionality is adequate when compared to other code editors. However, I would appreciate hearing your thoughts on this matter.

affvalente commented 1 year ago

Edit: what you describe seems consistent with how GitHub shows code. Perhaps we need an horizontal scrollbar

rajarshisamaddar commented 1 year ago

Edit: what you describe seems consistent with how GitHub shows code. Perhaps we need an horizontal scrollbar

The horizontal scroll bar is already present, anything else?

affvalente commented 1 year ago

Up to the team - I'm just a user!

rajarshisamaddar commented 1 year ago

@acywatson Would you mind reviewing this pull request, please?

rajarshisamaddar commented 1 year ago

Yea this looks right to me. Thanks!

Hi @acywatson, you're welcome. Could you please merge this request, if possible?

acywatson commented 1 year ago

Yea this looks right to me. Thanks!

Hi @acywatson, you're welcome. Could you please merge this request, if possible?

I'm actually second guessing this now - give me some time to think about this. Thank you

rajarshisamaddar commented 1 year ago

Yea this looks right to me. Thanks!

Hi @acywatson, you're welcome. Could you please merge this request, if possible?

I'm actually second guessing this now - give me some time to think about this. Thank you

Certainly! Take your time to think things through and let me know if you have any questions or if there's anything else I can help you with.

rajarshisamaddar commented 1 year ago

@zurfyx Please review this PR for #4361

zignis commented 1 year ago

Adding nowrap defeats the entire purpose, imo, as it will force horizontal scrollbars unless you hide the overflow, which is not desirable. The current need is to fix the line numbers while allowing the code to wrap.

thegreatercurve commented 11 months ago

Closing due to inactivity, but happy to re-open if comments above can be addressed.