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

MD: fix page controller updates #468

Merged
merged 1 commit into from Jan 7, 2023
Merged

MD: fix page controller updates #468

merged 1 commit into from Jan 7, 2023

Conversation

jpnurmi
Copy link
Member

@jpnurmi jpnurmi commented Jan 7, 2023

MasterDetailPage can be rebuilt with either a) a different controller, or b) a different length and initialIndex. Make sure didUpdateWidget checks for both cases.

Ref: ubuntu-flutter-community/settings#421

Copy link
Member

@Feichtmeier Feichtmeier left a comment

Choose a reason for hiding this comment

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

Thank you! Maybe you were right after all and we should just add a required controller some day 🤠

@jpnurmi
Copy link
Member Author

jpnurmi commented Jan 7, 2023

It was the opposite, actually. 🙈 A controller was required in the beginning because it made our internal implementation simple. However, it was inconvenient to use because often you don't really need one. Therefore I proposed making it easier for the user by allowing them to pass an initial index and length just like they did before, at the cost of making our internal implementation more complex...

@Feichtmeier Feichtmeier merged commit 5224bf2 into ubuntu:main Jan 7, 2023
@jpnurmi jpnurmi deleted the md-page-controller branch January 7, 2023 09:14
Feichtmeier pushed a commit that referenced this pull request Jan 7, 2023
Same as #468 but for NavigationPage.
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