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

Add navigate to view group action #860

Merged
merged 9 commits into from
Nov 6, 2023
Merged

Add navigate to view group action #860

merged 9 commits into from
Nov 6, 2023

Conversation

vinothvino42
Copy link
Collaborator

@vinothvino42 vinothvino42 commented Oct 5, 2023

Button:
  label: Navigate To Cast
  onTap:
    navigateViewGroup:
      viewIndex: 2

Video:
https://drive.google.com/file/d/1HLBquNs0E1fHlIjyNnKDNFhegcma2V35/view?usp=share_link

"properties": {
"navigateViewGroup": {
"type": "object",
"description": "Navigate between the screens in the view group",
Copy link
Contributor

Choose a reason for hiding this comment

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

add " (applicable only when inside a ViewGroup)"

@@ -113,6 +126,14 @@ class PageGroupState extends State<PageGroup> with MediaQueryCapability {

@override
Widget build(BuildContext context) {
return ListenableBuilder(
Copy link
Contributor

Choose a reason for hiding this comment

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

This will recreate the entire ViewGroup from scratch - we don't want that. We simply want to switch the Tab to another screen. Please make sure we don't rebuild items unnecessary. Optimization is important for our framework as the app could be gigantic.

@@ -125,16 +146,19 @@ class PageGroupState extends State<PageGroup> with MediaQueryCapability {
Drawer? drawer = _buildDrawer(context, widget.menu);
bool atStart = (widget.menu as DrawerMenu).atStart;
return PageGroupWidget(
key: UniqueKey(),
Copy link
Contributor

Choose a reason for hiding this comment

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

don't do this. Almost never use UniqueKey().

@vinothvino42
Copy link
Collaborator Author

vinothvino42 commented Oct 13, 2023

@vusters Can you review it again? Added UniqueKey only in the EnsembleBottomAppBar widget, it's not rebuilding the EnsembleBottomAppBar when the ListenableBuilder called the builder method with index.

@@ -99,6 +112,10 @@ class _BottomNavPageGroupState extends State<BottomNavPageGroup>
floatingAlignment =
FloatingAlignment.values.byName(fabMenuItem!.floatingAlignment);
}

WidgetsBinding.instance.addPostFrameCallback((_) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This is suboptimal. Why let it render, then re-render the correct page? Don't use this callback please.

@vinothvino42 vinothvino42 changed the title WIP: Add navigate to view group action Add navigate to view group action Nov 6, 2023
@vinothvino42 vinothvino42 merged commit 628e381 into main Nov 6, 2023
2 checks passed
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.

2 participants