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

[Breaking] Celllayout: fit the grid when layouting the root layoutable container #610

Draft
wants to merge 1 commit into
base: maintenance/mps20211
Choose a base branch
from

Conversation

alexanderpann
Copy link
Collaborator

@alexanderpann alexanderpann commented Apr 15, 2023

With this change, the width of cells with pushX/growX is the maximum of the text width (preferences->Editor->MPS Editor->text width) and the widest cell in the editor. The text width was not considered on purpose in the past. That means that, for example, horizontal lines had the width of the widest cell in the editor which doesn't make any sense to me. What's the point of having a maximum width for the editor if it is not always considered in the maximum width calculation? Was there any reason to disable the fitting for the preferred inner size? I couldn't find any examples where this causes layout issues, so I changed it.

Before:
Screenshot 2023-04-15 at 20 05 54
After:
Screenshot 2023-04-15 at 20 13 24

The lines might appear a bit long in this example because I have set the text width to 180. With a more standard text width like 100 it looks normal.

Screenshot 2023-04-15 at 20 06 38

You can even restore the old behavior by placing pushX/growX: false commands at specific levels in the editor. In the following example, the content collection of the test suite has the flags set, which makes the red lines the width of the content. The black line after the header has the full editor text width:

Screenshot 2023-04-15 at 22 11 41

@slisson
Copy link
Collaborator

slisson commented Apr 15, 2023

I'm not sure if this behavior is always desired. At least changing the return value of getPreferredInnerSize is not correct, but just a hack. Somewhere in the LayoutEngine class would be the right place for this change, preferably configurable.

@alexanderpann alexanderpann marked this pull request as draft April 16, 2023 08:42
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.

None yet

2 participants