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

Add possible solutions to fix text overrun #37

Closed

Conversation

JiaXinEu
Copy link
Contributor

@JiaXinEu JiaXinEu commented Jul 5, 2024

As discussed in #22
Another alternative (and more concise solution) for this is in PR #44

@laney0808
Copy link
Contributor

I think this PR could bring better learning outcome than #44 with more alternatives. But to achieve this we need more elaboration, such as a side-by-side explanation mentioned in conversation in #44.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, useful for students to figure out how to remove the truncated text, but some elaborations need to be made if we want students to understand the rationale behind the changes.

Comment on lines +118 to +123
<b>1. Changing MainWindow.fxml</b>

Remove `prefHeight` for `VBox`:
```xml
<VBox fx:id="dialogContainer" prefWidth="388.0" />
```

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe an explanation like:

This modification removes the preferred height setting for the VBox container. By not specifying a prefHeight, the VBox is allowed to expand vertically as needed based on its contents. This helps accommodate labels that might need more vertical space due to wrapped text, preventing the text from being cut off and ending with ....

Would be appropriate so that students know the impacts of the changes they are making.

Comment on lines +125 to +130
<b>2. Changing DialogBox.fxml</b>

Set `minHeight` for `Label`:
```xml
<Label fx:id="dialog" text="Label" wrapText="true" minHeight="-Infinity"/>
```

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this change, an explanation along the lines of:

This setting allows the label to have no minimum height constraint, meaning it can shrink or expand vertically as much as needed. This flexibility ensures that if the text wrapped within the label requires more height due to wrapping, the label can expand to fit this text without truncating it. This change is particularly useful in ensuring that all content of the label is visible, regardless of the amount of text.

Would be fitting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion, this issue has been resolved in PR #44

@damithc
Copy link
Contributor

damithc commented Jul 30, 2024

Let's close this, as #44 has been merged already.

@damithc damithc closed this Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants