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

state.value = newIndex is not causing a re-build of the widget even though newIndex is different #434

Open
nateshmbhat opened this issue Jun 18, 2024 · 4 comments
Assignees
Labels
bug Something isn't working question Further information is requested

Comments

@nateshmbhat
Copy link

nateshmbhat commented Jun 18, 2024

Describe the bug
Calling activeIndex.value = newIndex is not causing a re-build of the widget.

To Reproduce
Full Reproducible code :

class PracticeDetailScreen extends HookConsumerWidget {
  final YogaPracticeId practiceId;
  const PracticeDetailScreen({super.key, required this.practiceId});

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final practiceData = getYogaPracticeData(practiceId);
    final activeIndex = useState<int>(0);

    if (practiceData == null) {
      return const SAppScaffold(
        body: Center(
          child: Text('Practice not found'),
        ),
      );
    }
    return SAppScaffold(
      appBarOptions: SAppBarOptions(title: practiceData.title),
      body: SafeArea(
        child: Column(
          children: [
            TabBarRow(onTabChanged: (newIndex) {
              activeIndex.value = newIndex;
            }),
            activeIndex.value == 0
                ? const IntroTabView()
                : const MeditateTabView()
          ],
        ),
      ),
    );
  }
}

class TabBarRow extends HookConsumerWidget {
  const TabBarRow({super.key, required this.onTabChanged});
  final Function(int) onTabChanged;

  @override
  Widget build(BuildContext context, WidgetRef ref) {
    final activeIndex = useState<int>(0);
    useEffect(() {
      onTabChanged(activeIndex.value);
    }, [activeIndex.value]);

    return Container(
      margin: const EdgeInsets.symmetric(horizontal: 20),
      height: 50,
      padding: const EdgeInsets.symmetric(horizontal: 5),
      decoration: ShapeDecoration(
        color: Colors.white,
        shape: RoundedRectangleBorder(
          side: const BorderSide(width: 1, color: Color(0xFFE8EAEB)),
          borderRadius: BorderRadius.circular(6),
        ),
      ),
      child: Row(
        mainAxisSize: MainAxisSize.min,
        mainAxisAlignment: MainAxisAlignment.center,
        crossAxisAlignment: CrossAxisAlignment.center,
        children: [
          Expanded(
            child: TabBarButton(
              onPressed: () {
                activeIndex.value = 0;
              },
              isActive: activeIndex.value == 0,
              title: 'Intro',
            ),
          ),
          Expanded(
            child: TabBarButton(
                onPressed: () {
                  activeIndex.value = 1;
                },
                isActive: activeIndex.value == 1,
                title: 'Meditate'),
          ),
        ],
      ),
    );
  }
}

class TabBarButton extends StatelessWidget {
  const TabBarButton(
      {super.key,
      required this.onPressed,
      required this.isActive,
      this.title = 'Intro'});

  final bool isActive;
  final VoidCallback onPressed;
  final String title;

  @override
  Widget build(BuildContext context) {
    return SPressable(
      onPressed: () {
        onPressed();
      },
      child: Container(
        height: 40,
        padding: const EdgeInsets.symmetric(horizontal: 14, vertical: 10),
        decoration: ShapeDecoration(
          color: isActive ? const Color(0xFF1FA1AA) : null,
          shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(5)),
        ),
        child: Column(
          mainAxisSize: MainAxisSize.min,
          mainAxisAlignment: MainAxisAlignment.center,
          crossAxisAlignment: CrossAxisAlignment.center,
          children: [
            Text(
              title,
              style: TextStyle(
                color: isActive ? Colors.white : SColors.darkGrey,
                fontSize: 14,
                fontFamily: 'Open Sans',
                fontWeight: FontWeight.w600,
                height: 0,
              ),
            ),
          ],
        ),
      ),
    );
  }
}

class IntroTabView extends HookConsumerWidget {
  const IntroTabView({super.key});
  @override
  Widget build(BuildContext context, WidgetRef ref) {
    return Container(
      height: 100,
      width: 50,
      color: Colors.blue,
      child: const Text('intro'),
    );
  }
}

class MeditateTabView extends HookConsumerWidget {
  const MeditateTabView({super.key});
  @override
  Widget build(BuildContext context, WidgetRef ref) {
    return Container(child: const Text('meditate'));
  }
}

Package versions :
hooks_riverpod: ^2.5.1
flutter_hooks: ^0.20.5
riverpod_annotation: ^2.3.5

Expected behavior
Clicking on the "Intro" button or "Meditate" button should show the corresponding IntroTabView and MeditateTabView .

@nateshmbhat nateshmbhat added bug Something isn't working needs triage labels Jun 18, 2024
@nateshmbhat nateshmbhat changed the title state.value = newIndex is not causing a re-build of the widget Jun 20, 2024
@nateshmbhat
Copy link
Author

Can u please check on this? This looks like a critical issue in useState...

@rrousselGit
Copy link
Owner

I'm in holiday. But try:

    useEffect(() {
      Future(() => onTabChanged(activeIndex.value));
    }, [activeIndex.value]);

I'm pretty sure you have an exception somewhere in your code because of this useEffect.

It's violating some widget rules, which would lead to Flutter not updating the UI.

@rrousselGit rrousselGit added question Further information is requested and removed needs triage labels Jun 26, 2024
@nateshmbhat
Copy link
Author

Can someone pls tell me which widget rules is violated? This is the full code I pasted which can be run in any project.

@Pablets
Copy link

Pablets commented Jun 26, 2024

I hardcoded missing data on your example because it's not reproducible as it is.

When I make it run and press on the tab buttons this error appears:

Exception has occurred.
FlutterError (setState() or markNeedsBuild() called during build.
This PracticeDetailScreen widget cannot be marked as needing to build because the framework is already in the process of building widgets. A widget can be marked as needing to be built during the build phase only if one of its ancestors is currently building. This exception is allowed because the framework builds parent widgets before children, which means a dirty descendant will always be built. Otherwise, the framework might not visit this widget during this build phase.
The widget on which setState() or markNeedsBuild() was called was:
  PracticeDetailScreen
The widget which was currently being built when the offending call was made was:
  TabBarRow)

So, when PracticeDetailScreen is still building there is another child widget triggering a rebuild in PracticeDetailScreen again.

This code "just works" meaning that I'm not 100% sure if these hooks are meant to be used like that, sorry I come from react and I din't find any documentation about this but the reasoning is this:

The state must be in one parent widget (PracticeDetailScreen) and be passed down to your child widgets as props.
Instead you are making a new state inside TabBarRow and somehow (I didn't understand that part of the
implementation) trying to sinchronize it with the parent state via a useEffect and a funtion.

and the code is this:

import 'package:flutter/material.dart';
import 'package:flutter_hooks/flutter_hooks.dart';
import 'package:hooks_riverpod/hooks_riverpod.dart';

class PracticeDetailScreen extends HookConsumerWidget {
 // I don't know what `YogaPracticeId` is so I leave it hardcoded.
 final int practiceId;
 // I put a default value, with 0 you trigger a empty response.
 const PracticeDetailScreen({super.key, this.practiceId = 1});

 @override
 Widget build(BuildContext context, WidgetRef ref) {
   // The data it's not important in this case so we can leave it hardcoded
   final practiceData = practiceId != 0 ? null : 'some data';
   final activeIndex = useState<int>(0);

   if (practiceData == null) {
     return const Scaffold(
       body: Center(
         child: Text('Practice not found'),
       ),
     );
   }
   // Removed unncessesary dependencies
   return Scaffold(
     body: SafeArea(
       child: Column(
         children: [
           TabBarRow(activeIndex: activeIndex),
           activeIndex.value == 0
               ? const IntroTabView()
               : const MeditateTabView()
         ],
       ),
     ),
   );
 }
}

class TabBarRow extends StatelessWidget {
 // Here we receive the state `activeIndex` from the parent that is of type  ValueNotifier<int>
 final ValueNotifier<int> activeIndex;
 const TabBarRow({super.key, required this.activeIndex});

 @override
 Widget build(BuildContext context) {
   return Container(
     margin: const EdgeInsets.symmetric(horizontal: 20),
     height: 50,
     padding: const EdgeInsets.symmetric(horizontal: 5),
     decoration: ShapeDecoration(
       color: Colors.white,
       shape: RoundedRectangleBorder(
         side: const BorderSide(width: 1, color: Color(0xFFE8EAEB)),
         borderRadius: BorderRadius.circular(6),
       ),
     ),
     child: Row(
       mainAxisSize: MainAxisSize.min,
       mainAxisAlignment: MainAxisAlignment.center,
       crossAxisAlignment: CrossAxisAlignment.center,
       children: [
         Expanded(
           child: TabBarButton(
             onPressed: () {
               activeIndex.value = 0;
             },
             isActive: activeIndex.value == 0,
             title: 'Intro',
           ),
         ),
         Expanded(
           child: TabBarButton(
               onPressed: () {
                 activeIndex.value = 1;
               },
               isActive: activeIndex.value == 1,
               title: 'Meditate'),
         ),
       ],
     ),
   );
 }
}

class TabBarButton extends StatelessWidget {
 const TabBarButton(
     {super.key,
     required this.onPressed,
     required this.isActive,
     this.title = 'Intro'});

 final bool isActive;
 final VoidCallback onPressed;
 final String title;

 @override
 Widget build(BuildContext context) {
   return ElevatedButton(
     onPressed: () {
       onPressed();
     },
     child: Container(
       height: 40,
       padding: const EdgeInsets.symmetric(horizontal: 14, vertical: 10),
       decoration: ShapeDecoration(
         color: isActive ? const Color(0xFF1FA1AA) : null,
         shape: RoundedRectangleBorder(borderRadius: BorderRadius.circular(5)),
       ),
       child: Column(
         mainAxisSize: MainAxisSize.min,
         mainAxisAlignment: MainAxisAlignment.center,
         crossAxisAlignment: CrossAxisAlignment.center,
         children: [
           Text(
             title,
             style: TextStyle(
               color: isActive ? Colors.white : Colors.grey,
               fontSize: 14,
               fontFamily: 'Open Sans',
               fontWeight: FontWeight.w600,
               height: 0,
             ),
           ),
         ],
       ),
     ),
   );
 }
}

class IntroTabView extends HookConsumerWidget {
 const IntroTabView({super.key});
 @override
 Widget build(BuildContext context, WidgetRef ref) {
   return Container(
     height: 100,
     width: 50,
     color: Colors.blue,
     child: const Text('intro'),
   );
 }
}

class MeditateTabView extends HookConsumerWidget {
 const MeditateTabView({super.key});
 @override
 Widget build(BuildContext context, WidgetRef ref) {
   return const Text('meditate');
 }
}

I would investigate a bit more if I were you, maybe some experienced flutter dev can help, but in the meantime it works.
If this approach have unknown pitfalls then I think the other option is to use a provider.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working question Further information is requested
3 participants