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.

Sign up for GitHub

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

Jump to bottom

Add YaruMasterDetailPaneLayoutDelegate #403

Merged
merged 6 commits into from
Nov 20, 2022
Merged

Add YaruMasterDetailPaneLayoutDelegate #403

merged 6 commits into from
Nov 20, 2022

Conversation

Jupi007
Copy link
Member

@Jupi007 Jupi007 commented Nov 19, 2022

Add YaruMasterDetailPaneLayoutDelegate, which controls a YaruMasterDetailPage pane with defined parameters.
This force the user to think about layout size constraints of.

Fixes #393

Pull request checklist

  • This PR does not introduce visual changes

@Jupi007 Jupi007 requested a review from jpnurmi November 19, 2022 16:28
@jpnurmi
Copy link
Member

jpnurmi commented Nov 20, 2022

This is great! Is there any way to come up with a reasonable fixed default? Was the former leftPaneWidth always a required property?

@Feichtmeier
Copy link
Member

This is great! Is there any way to come up with a reasonable fixed default? Was the former leftPaneWidth always a required property?

Yeah it was :D I also vote for a default :) The less to configure the better

@Jupi007
Copy link
Member Author

Jupi007 commented Nov 20, 2022

I added a default pane width to both delegate :)

jpnurmi
jpnurmi reviewed Nov 20, 2022
View reviewed changes
Copy link
Member

@jpnurmi jpnurmi left a comment •
edited
Loading

Choose a reason for hiding this comment

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

i'd keep the delegate properties required and assign a default layout delegate instead. this makes it very convenient to get started with some reasonable default. if the user is not happy with the default behavior, they can create a custom delegate but then they are forced to enter all relevant values to avoid surprises.

lib/src/layouts/yaru_master_detail_page.dart Outdated Show resolved Hide resolved
lib/src/layouts/yaru_master_detail_layout_delegate.dart Outdated Show resolved Hide resolved
lib/src/layouts/yaru_master_detail_layout_delegate.dart Outdated Show resolved Hide resolved
@Jupi007
Copy link
Member Author

Jupi007 commented Nov 20, 2022

Ohh, okay!
Good idea 👍

jpnurmi
jpnurmi approved these changes Nov 20, 2022
View reviewed changes
Copy link
Member

@jpnurmi jpnurmi left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM 👍

@Jupi007 Jupi007 merged commit 8465a62 into main Nov 20, 2022
@Jupi007 Jupi007 deleted the Jupi007/issue393 branch November 20, 2022 14:09
@Jupi007
Copy link
Member Author

Jupi007 commented Nov 20, 2022

And thank you very much for the idea! 🙂

@jpnurmi jpnurmi mentioned this pull request Nov 22, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpnurmi jpnurmi jpnurmi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add YaruMasterDetailResizeDelegate
3 participants
@Jupi007 @jpnurmi @Feichtmeier