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

YaruPageIndicator: add custom scale parameters #624

Merged
merged 4 commits into from Feb 22, 2023

Conversation

d-loose
Copy link
Member

@d-loose d-loose commented Feb 22, 2023

Allows to customize the size and spacing of YaruPageIndicator's dots:

Screencast.from.2023-02-22.12-48-53.webm

Fix #622

Pull request checklist

  • This PR does not introduce visual changes, or
    • I ran flutter test --update-goldens and committed the changes if there were any, or
    • I added before/after/light/dark screenshots if the visual changes I made were not covered by golden tests.
      Before After
      Light
      Dark

@Feichtmeier
Copy link
Member

Looks good because it works 😄

As I do not care enough for the implementation detail let @jpnurmi and @Jupi007 double check as they are usually a bit more nit-picky

@Feichtmeier Feichtmeier requested review from jpnurmi and Jupi007 and removed request for Feichtmeier February 22, 2023 12:08
Copy link
Member

@Jupi007 Jupi007 left a comment

Choose a reason for hiding this comment

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

I agree that provide a way to customize dot size and spacing is a great idea!

But the way I implemented the current spacing was preventing layout overflow, while this PR doesn't provide any kind of security:

Peek.22-02-2023.13-25.mp4

@d-loose Can you implement a way to avoid overflow?

Also it would be nice that your new properties could not be factors, but real size:

final double dotSize = 12.0;
final double dotSpacingTarget; // target, because it can be stretched cause to the overflow guard

It is more intuitive in that way, and so we don't hide those parameters with some obscure internal stuff.

@jpnurmi
Copy link
Member

jpnurmi commented Feb 22, 2023

As a user, I'd rather specify the desired size and spacing than a factor because I don't know what the factor applies to :)

@d-loose
Copy link
Member Author

d-loose commented Feb 22, 2023

Thanks!
Vertical constraints are respected again, and I've changed the parameters to accept absolute values.

Screencast.from.2023-02-22.13-51-39.webm

@d-loose d-loose requested a review from Jupi007 February 22, 2023 12:53
Copy link
Member

@Jupi007 Jupi007 left a comment

Choose a reason for hiding this comment

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

I can't actually test it, but if you ensure me there's no more vertical overflow, then LGFM 🙂 👍

@d-loose
Copy link
Member Author

d-loose commented Feb 22, 2023

I can :)
It only happened before, because I recklessly rescaled maxWidth 😆

@d-loose d-loose merged commit 7da61e0 into ubuntu:main Feb 22, 2023
@d-loose d-loose deleted the page-indicator-scale branch February 22, 2023 13:13
@Feichtmeier Feichtmeier mentioned this pull request Feb 22, 2023
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.

YaruPageIndicator: allow customization?
4 participants