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

Remove margin around TextEditor #419

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

mgarstenauer
Copy link
Contributor

This is a cosmetic PR that removes the 30px margin around the TextEditor in the demo.

Before:
image

After:
image

(Looking at the git history, the Margin has been there for the last 7+ years. I don't think that the margin serves any purpose. Let's remove it to avoid any confusion!)

@Rekkonnect
Copy link

Most code editors have a margin on the line number panel to make it more readable and prettier. Removing the margin feels much worse to me.

@mgarstenauer
Copy link
Contributor Author

@Rekkonnect Thanks for the feedback. I was mainly worried about the margins on the other sides. Having space between the scrollbars and the window, felt like a bug.

I pushed a small change. Keep some margin on the left for readability, remove the top/right/bottom margins. Here is what it looks like:

image

Increase the size of the LineNumberMargin to improve readability and
ensure that there is some space between the line numbers and the next
AbstractMargin.
@mgarstenauer mgarstenauer force-pushed the remove-margin-around-texteditor branch from 35b1b41 to bbd2e65 Compare May 31, 2024 14:01
@mgarstenauer
Copy link
Contributor Author

mgarstenauer commented May 31, 2024

I increased the minimum size of the LineNumberMargin. This ensures that there is enough space between the line numbers and any other text editor margins.

Together with the breakpoint margin it looks like this:
image

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.

2 participants