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

Feedback request #11

Closed
talamaska opened this issue Sep 29, 2021 · 10 comments
Closed

Feedback request #11

talamaska opened this issue Sep 29, 2021 · 10 comments
Labels
help wanted Extra attention is needed

Comments

@talamaska
Copy link
Owner

talamaska commented Sep 29, 2021

So here is the deal, in a recent update adding the arrows for the label box, I moved the rendering of the text in a painter. This has some implications, like text overflow, max lines, text direction and positioning of the text. I have been doing this with ParagraphBuilder, Now, I made some experiments to make the same thing work with TextPainter and the results are plausible, I need it to calculate the correct size of the label box size and padding.
In the end I think I want to avoid dealing with all the quirks when painting and positioning texts. I also think it is not so performant as using plain Text widgets placed in a Column. I'm wondering is it a good idea to limit maxLines for title and the body and work with the TextOverflow to avoid Flutter overflow errors.
I do realize that words could be way longer in different languages and I'm still not sure how to deal with.
Maybe loosening the maxLines and still rely on the TextOverflow? Like Title - 2 lines, body - 5, but if there isn't space for 5 lines display 3 and add ellipsis. Or maybe add scroll to the label box. I honestly think adding scroll to the text in the overlay would be weird UX.
I'm open to discussion here. I can't promise I'll comply to all suggestion, but at least I want to get some user's feedback.

Edit: you can take a look and the suggested code changes here
https://pub.dev/packages/onboarding_overlay/versions/2.3.1-pre.2
or in the branch https://github.com/talamaska/onboarding_overlay/tree/feature/add-forwarding-events

@Macacoazul01
Copy link

You could use AutoSizeText to handle with the majority of the overflows:

https://pub.dev/packages/auto_size_text

Only if the text is real big this solution won't work (and if that's the case, the dev should rethink it, as it isn't ideal to have a big onboarding text )

The downside is adding a dependency to the package

@talamaska
Copy link
Owner Author

auto-size is great, but it feels a bit of a magic for the end user and also could lead to unexpected results since we are able to set style for the title and the body. Also I'm not sure if it will not lead to some performance regression. Thanks for the suggestion.

@Macacoazul01
Copy link

The end user won't know if it was used autosize or the normal text and the configs for both are the same. For the dev it will help when the user has a smaller screen than expected (and the dev can decide the minimum size of the text to avoid weird behaviors). About the performance i really don't know the impact.

@Macacoazul01
Copy link

You could try something like adding an option autosize: true/false (using false as default) so then the dev chooses if he wishes an autosized or normal text.

@talamaska
Copy link
Owner Author

I like that idea, just fyi, when I said end-user I meant the the dev using the package. But you are right, this could save me from a whole lot of problems with smaller screen sizes.

@talamaska
Copy link
Owner Author

talamaska commented Oct 3, 2021

Suggested code changes are here
https://pub.dev/packages/onboarding_overlay/versions/2.3.1-pre.3
or in the branch https://github.com/talamaska/onboarding_overlay/tree/feature/add-forwarding-events

Added new global property for the whole Onboarding autoSizeTexts with default value false. So when it is false I fallback to maxLines for title equal to 2 and maxLines for body text equal to 5.

I think it turned out pretty well and I was able to remove some painting code as well.

@Macacoazul01
Copy link

@talamaska is there anything missing for v2.3.1 becoming stable?

@talamaska
Copy link
Owner Author

talamaska commented Oct 14, 2021

@Macacoazul01 Yes, it seems that auto_size_text null-safety version is a pre-release version.
I'm getting a warning when i want to publish my package as stable release because of my dependency of the prerelease package.
I wonder if i can ignore it. i have requested the dev to publish to stable version, but I'm not sure when this will happen, which is a bummer.
simc/auto_size_text#99

@talamaska
Copy link
Owner Author

will wait couple of days to see if there is a reaction from the maintainer and if not will fork and publish the fork.

@talamaska
Copy link
Owner Author

@Macacoazul01 released stable 2.3.1

@talamaska talamaska added the help wanted Extra attention is needed label Dec 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants