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

implement word wrapping functionality #755

Merged
merged 16 commits into from
Nov 6, 2024

Conversation

nnyyxxxx
Copy link
Contributor

@nnyyxxxx nnyyxxxx commented Oct 4, 2024

@ChrisTitusTech this pr adds word wrapping to descriptions and the important actions guide, it can be enabled for previews as well if you want.

Type of Change

  • New feature

Description

adds dynamic / auto updating word wrapping to the descriptions

@nnyyxxxx nnyyxxxx changed the title implement dynamic auto updating word wrapping functionality implement dynamic auto updating word wrapping functionality to the descriptions Oct 4, 2024
Copy link
Collaborator

@adamperkowski adamperkowski left a comment

Choose a reason for hiding this comment

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

Looks OK from a first glance.

@nnyyxxxx nnyyxxxx marked this pull request as draft October 31, 2024 19:08
@adamperkowski adamperkowski changed the title implement dynamic auto updating word wrapping functionality to the descriptions ui (descriptions): implement dynamic auto updating word wrapping functionality Oct 31, 2024
@nnyyxxxx nnyyxxxx marked this pull request as ready for review October 31, 2024 19:25
@afonsofrancof
Copy link
Collaborator

I removed the FloatingTextMode a few days ago, so we can use any title we want.
Maybe it's better to have the new() function receive a boolean telling it if it should wrap the words, and we only activate that on the description for now.
It would then be very easy to activate it in the future for other float types

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 5, 2024

I removed the FloatingTextMode a few days ago, so we can use any title we want. Maybe it's better to have the new() function receive a boolean telling it if it should wrap the words, and we only activate that on the description for now. It would then be very easy to activate it in the future for other float types

that's already implemented via enum rather than boolean

@afonsofrancof
Copy link
Collaborator

afonsofrancof commented Nov 5, 2024

that's already implemented via enum rather than boolean

Yes, I know, that enum existed a few days ago, and it was removed to make the float more generic, so that the title can be anything instead of just those 3 enum types.

That's why I was saying that a boolean is better.

Check the current implementation and you will understand.

Why should we restrict the float to just those 3 types when all we need is to either wrap or not?

@nnyyxxxx
Copy link
Contributor Author

nnyyxxxx commented Nov 5, 2024

that's already implemented via enum rather than boolean

Yes, I know, that enum existed a few days ago, and it was removed to make the float more generic, so that the title can be anything instead of just those 3 enum types.

That's why I was saying that a boolean is better.

oh i didnt notice

@nnyyxxxx nnyyxxxx marked this pull request as draft November 5, 2024 21:27
@nnyyxxxx nnyyxxxx marked this pull request as ready for review November 5, 2024 21:44
@nnyyxxxx nnyyxxxx changed the title ui (descriptions): implement dynamic auto updating word wrapping functionality implement word wrapping functionality Nov 5, 2024
Copy link
Collaborator

@afonsofrancof afonsofrancof left a comment

Choose a reason for hiding this comment

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

lgtm

@ChrisTitusTech ChrisTitusTech merged commit 565f507 into ChrisTitusTech:main Nov 6, 2024
3 checks passed
@nnyyxxxx nnyyxxxx deleted the wordwrap branch November 6, 2024 18:44
@nnyyxxxx nnyyxxxx mentioned this pull request Nov 6, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants