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

fix(YaruMasterTile): let onTap override scope ontap #738

Merged
merged 1 commit into from Jul 27, 2023

Conversation

Feichtmeier
Copy link
Member

@Feichtmeier Feichtmeier commented Jul 27, 2023

This adds a parameter to disable looking up the YaruMasterTileScope for tiles added to the list inside YMD that should not open a page.

Is this the correct approach @jpnurmi ?

Use case: 1 tile inside the list should open a dialog and not select/load a page

Closes #718

@Feichtmeier Feichtmeier requested a review from jpnurmi July 27, 2023 11:40
@jpnurmi
Copy link
Member

jpnurmi commented Jul 27, 2023

What's the problem you're trying to solve? YaruMasterTileScope.maybeOf() returns null if it's not wrapped into a scope.

@Feichtmeier
Copy link
Member Author

I think you commented 2 secs before I updated the PR desciption :P

This is the use case, X tiles inside the list should not open a page but just onTap:()=> doSomething()

Bildschirmaufzeichnung.vom.2023-07-27.13-41-28.webm

@jpnurmi
Copy link
Member

jpnurmi commented Jul 27, 2023

What if we'd allow YaruMasterTile.onTap() "override" the scope i.e. if you specify your own onTap handler then the scope handler won't be called?

diff --git a/lib/src/widgets/master_detail/yaru_master_tile.dart b/lib/src/widgets/master_detail/yaru_master_tile.dart
index f655985..5ba8a32 100644
--- a/lib/src/widgets/master_detail/yaru_master_tile.dart
+++ b/lib/src/widgets/master_detail/yaru_master_tile.dart
@@ -76,8 +76,11 @@ class YaruMasterTile extends StatelessWidget {
           trailing: trailing,
           selected: isSelected,
           onTap: () {
-            scope?.onTap();
-            onTap?.call();
+            if (onTap != null) {
+              onTap!.call();
+            } else {
+              scope?.onTap();
+            }
           },
         ),
       ),

@Feichtmeier
Copy link
Member Author

What if we'd allow YaruMasterTile.onTap() "override" the scope i.e. if you specify your own onTap handler then the scope handler won't be called?

diff --git a/lib/src/widgets/master_detail/yaru_master_tile.dart b/lib/src/widgets/master_detail/yaru_master_tile.dart
index f655985..5ba8a32 100644
--- a/lib/src/widgets/master_detail/yaru_master_tile.dart
+++ b/lib/src/widgets/master_detail/yaru_master_tile.dart
@@ -76,8 +76,11 @@ class YaruMasterTile extends StatelessWidget {
           trailing: trailing,
           selected: isSelected,
           onTap: () {
-            scope?.onTap();
-            onTap?.call();
+            if (onTap != null) {
+              onTap!.call();
+            } else {
+              scope?.onTap();
+            }
           },
         ),
       ),

yeah that would be even better, that was funnily what I assumed would happen when you override ontap 😄

this would also fix #718 btw

@Feichtmeier Feichtmeier changed the title feat(YaruMasterTile): add lookUpScope parameter fix(YaruMasterTile): let onTap override scope ontap Jul 27, 2023
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.

LGTM 👍

@Feichtmeier Feichtmeier enabled auto-merge (squash) July 27, 2023 11:55
@Feichtmeier Feichtmeier merged commit 9ac8623 into main Jul 27, 2023
9 checks passed
@Feichtmeier Feichtmeier deleted the yaru_master_tile_scope_off branch July 27, 2023 11:56
@github-actions github-actions bot mentioned this pull request Jul 27, 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.

Add a navigation focused alternative to YaruMasterDetailPage
2 participants