Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

🐛 interference in _NavigationBodyState between PageView and paneBodyBuilder/IndexedStack #679

Closed
klondikedragon opened this issue Jan 14, 2023 · 2 comments · Fixed by #680
Labels
bug Something isn't working

Comments

@klondikedragon
Copy link
Contributor

Describe the bug
I've recently upgraded fluent_ui from 4.0.1 to 4.1.4, and started to receive all kinds of odd/random issues. e.g., duplicate GlobalKeys in widgets related to a nested vrouter, stateful widgets being disposed almost immediately after creation (causing futures executed in a "zero" delay created in an initState that reference the build context to get an exception that the widget was already detached), widgets not having a sized viewport during construction, etc.

It appears this is related to the recent enhancements (#607, #645) surrounding preserving state of navigation pane items through the ability to specify a key for each pane item. This is a great enhancement and good when the use of navigation pane is simple (common use case for most apps probably). In my case, I need to manage the keys/state of the widgets presented within the body of the navigation pane explicitly (and underneath an IndexedStack) because the content is not determined just by the currently selected pane, but also by the current route, which could include sub-routes that reference tabs with different content. (A sketch of what is being done is in #226 (comment) ).

It appears that using an IndexedStack in combination with paneBodyBuilder is contemplated (somewhat discussed in #649 (comment)).

I'm not exactly sure why, but the new PageView.builder (with an automatic GlobalKey passed to it) is really interfering strongly with the IndexedStack returned by my paneBodyBuilder.

I've tested a solution that seems to work nicely, where if the paneBodyBuilder is not null then it just doesn't wrap the widget built by the paneBodyBuilder in the PageView.builder (nor does it use the _pageKey in _NavigationBodyState) and that removes all of the problems.

I've created a PR for consideration to fix this issue and #678.

klondikedragon added a commit to klondikedragon/fluent_ui that referenced this issue Jan 14, 2023
A common use case for paneBodyBuilder is to use an IndexedStack to
manage the state of the widgets displayed inside the navigation body.
An IndexedStack does not play nicely with the PageView.builder that it
was being wrapped in. Now, if a paneBodyBuilder is used, assume it is
responsible for its own state management.

Fixes bdlukaa#679
klondikedragon added a commit to klondikedragon/fluent_ui that referenced this issue Jan 14, 2023
@bdlukaa bdlukaa added the bug Something isn't working label Jan 16, 2023
@b34st80y
Copy link

@klondikedragon Hello, I have been following this change and I am interested in using this implementation with IndexedStack (attempting to follow with #266 (comment) )

(I had a similar experience where the latest fluent_ui version broke my implementation)

however I am still experiencing some issues and have some questions...

See code sample below:

This code sample works ALMOST as intended using the latest fluent_ui code directly from master branch (including PR #680). The intention is to save the state between pages by use of the IndexedStack.

See comment "//--HERE--" above the NavigationView.selected property...

If this property is left as null - the IndexedStack and children hold state as desired BUT the NavPaneItems do not show when they are selected.
If this property is set to Index - the items DO show when they are selected BUT the entire IndexedStack and children are rebuilt when the index is changed.

It is confusing to me how this property can effect the state of NavigationView body like this. Is there a way I can accomplish my desired behavior of the IndexedStack and children staying-alive as desired AND have the items show when they are selected?

import 'package:fluent_ui/fluent_ui.dart';
import 'package:flutter/material.dart';

void main() {
  runApp(const MyApp());
}

class MyApp extends StatefulWidget {
  const MyApp({super.key});

  @override
  State<MyApp> createState() => _MyAppState();
}

class _MyAppState extends State<MyApp> {
  int index = 0;

  final pages = [
    const MyHomePage(title: "Page 1"),
    const MyHomePage(title: "Page 2"),
    const MyHomePage(title: "Page 3"),
  ];

  @override
  Widget build(BuildContext context) {
    return FluentApp(
      title: 'Flutter Demo',
      home: NavigationView(
        pane: NavigationPane(
          displayMode: PaneDisplayMode.top,
          // *HERE*
          selected: index,
          onChanged: (i) => setState(() => index = i),
          items: [
            PaneItem(
              icon: const Icon(FluentIcons.send),
              title: const Text('Page 1'),
              // placeholder because required
              body: Container(),
            ),
            PaneItem(
              icon: const Icon(FluentIcons.download),
              title: const Text('Page 2'),
              // placeholder because required
              body: Container(),
            ),
            PaneItem(
              icon: const Icon(FluentIcons.download),
              title: const Text('Page 3'),
              // placeholder because required
              body: Container(),
            ),
            PaneItemSeparator(),
          ],
        ),
        paneBodyBuilder: (body) => IndexedStack(
          index: index,
          children: pages,
        ),
      ),
    );
  }
}

class MyHomePage extends StatefulWidget {
  const MyHomePage({super.key, required this.title});

  final String title;

  @override
  State<MyHomePage> createState() => _MyHomePageState();
}

class _MyHomePageState extends State<MyHomePage> {
  int _counter = 0;

  void _incrementCounter() {
    setState(() {
      _counter++;
    });
  }

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(
        title: Text(widget.title),
      ),
      body: Center(
        child: Column(
          mainAxisAlignment: MainAxisAlignment.center,
          children: <Widget>[
            const Text(
              'You have pushed the button this many times:',
            ),
            Text('$_counter'),
          ],
        ),
      ),
      floatingActionButton: FloatingActionButton(
        onPressed: _incrementCounter,
        tooltip: 'Increment',
        child: const Icon(Icons.add),
      ),
    );
  }
}

@b34st80y
Copy link

Thank You!!! @bdlukaa @klondikedragon You are the GOAT. 🙏 🙏 🙏

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants