Skip to content

feat: Add lines option to control pyodide editor height #85

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

Closed
wants to merge 1 commit into from

Conversation

amit-s19
Copy link

@amit-s19 amit-s19 commented Mar 12, 2025

Closes #41

This PR adds a new lines option to pyodide fence editors that allows users to specify the initial number of visible lines. This helps save vertical space for small code examples and provides more room for larger ones.

Changes:

  • Add CSS support for custom editor height
  • Implement lines option in pyodide formatter
  • Add documentation with examples
  • Add unit tests

Example usage:

print("hello")
print("world")
print("!")

@pawamoy
Copy link
Owner

pawamoy commented Mar 13, 2025

Thanks a lot for the PR @amit-s19 🙂

The changes look good at first glance, however the solution is based on lines * 20px, which doesn't yield a good result: for example with lines="30", I only get 28 lines in the editor before it adds a scrollbar:

editor28

editor29

I suppose 20px is not the exact right value, and I worry no pixel value will ever be, since font-family, font-size, line height can change depending on many things on the client side 🤔

Instead, @waimea-cpy posted a demo where the Ace editor's maxLines and minLines options are used. I suppose using these options will be much more robust.

Finally, in the issue we discussed about having the height, min_height, max_height and resize options, while you only implemented lines here, so I wouldn't consider this PR complete 🙂 If you're willing to continue the work on this PR, I'm happy to continue my reviews 😄

@michaelweinold
Copy link

@amit-s19, are you still on this?

@amit-s19
Copy link
Author

Hey @michaelweinold yes, but I'll probably take a look at this tomorrow.

@pawamoy
Copy link
Owner

pawamoy commented Apr 10, 2025

I'll close this for now @amit-s19, let me know if you want to start working on this again.

@pawamoy pawamoy closed this Apr 10, 2025
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 this pull request may close these issues.

feature: Add option to specify initial number of lines in pyodide fence editors
3 participants