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: restore compatibility #672

Merged
merged 4 commits into from Mar 11, 2023
Merged

YaruPageIndicator: restore compatibility #672

merged 4 commits into from Mar 11, 2023

Conversation

jpnurmi
Copy link
Member

@jpnurmi jpnurmi commented Mar 11, 2023

Restore the old dotSize & dotSpacing arguments for the default constructor, and introduce an alternative named constructor for the new item size builder and layout delegate. This would be enough to avoid breaking the installer that is already using dotSize & dotSpacing. WDYT?

@jpnurmi jpnurmi requested a review from Jupi007 March 11, 2023 21:29
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.

Great idea!
I didn't thought about mitigate the backward compatibility.

@jpnurmi
Copy link
Member Author

jpnurmi commented Mar 11, 2023

What about the name? Do you like YaruPageIndicator.builder() or YaruPageIndicator.delegate() more? Perhaps the latter is actually better because the default constructor still takes one builder i.e. itemBuilder... 🤔

@Jupi007
Copy link
Member

Jupi007 commented Mar 11, 2023

Yeah, it makes sense to name it YaruPageIndicator.delegate() 👍

Edit: or drop the builder parameters from the default constructor, and leave their access in YaruPageIndicator.builder().

@jpnurmi
Copy link
Member Author

jpnurmi commented Mar 11, 2023

Edit: or drop the builder parameters from the default constructor, and leave their access in YaruPageIndicator.builder().

Great idea! 👍 I forgot it was also a new argument. Now the builder naming makes sense. :)

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.

Great, LGTM 👍

@jpnurmi jpnurmi merged commit 1ea44af into ubuntu:main Mar 11, 2023
@jpnurmi jpnurmi deleted the page-indicator-compat branch March 11, 2023 21:50
@jpnurmi jpnurmi mentioned this pull request Mar 11, 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.

None yet

2 participants