Skip to content

Conversation

@kranfix
Copy link
Contributor

@kranfix kranfix commented Aug 25, 2020

In the package example, the widget inspector for the useState example shows nothing about the hook state:

With this new feature, if these lines are added to the _StateHookState, this will be shown int the widget inspector.

class _StateHookState<T> extends HookState<ValueNotifier<T>, _StateHook<T>> {
  ValueNotifier<T> _state;

 ///...

  @override
  DiagnosticsProperty get diagnosticsProperty {
    return DiagnosticsProperty<T>('useState<$T>', _state.value);
  }
}

@codecov
Copy link

codecov bot commented Aug 25, 2020

Codecov Report

Merging #178 into master will increase coverage by 0.05%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #178      +/-   ##
==========================================
+ Coverage   99.62%   99.67%   +0.05%     
==========================================
  Files          10       10              
  Lines         531      616      +85     
==========================================
+ Hits          529      614      +85     
  Misses          2        2              
Impacted Files Coverage Δ
lib/src/animation.dart 100.00% <100.00%> (ø)
lib/src/async.dart 100.00% <100.00%> (ø)
lib/src/focus.dart 100.00% <100.00%> (ø)
lib/src/framework.dart 100.00% <100.00%> (ø)
lib/src/listenable.dart 100.00% <100.00%> (ø)
lib/src/misc.dart 98.33% <100.00%> (+0.41%) ⬆️
lib/src/primitives.dart 100.00% <100.00%> (ø)
lib/src/scroll_controller.dart 100.00% <100.00%> (ø)
lib/src/tab_controller.dart 100.00% <100.00%> (ø)
lib/src/text_controller.dart 95.23% <100.00%> (+0.23%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4054ed0...dd17638. Read the comment docs.

@rrousselGit
Copy link
Owner

Thanks!

We will need some tests for this.

@kranfix
Copy link
Contributor Author

kranfix commented Aug 25, 2020

According to the coverage/lcov.info, the HookElement.debugFillProperties is covered. Any suggestion for adding a local tests for this feature?


/// The logic and internal state for a [HookWidget]
abstract class HookState<R, T extends Hook<R>> {
abstract class HookState<R, T extends Hook<R>> with Diagnosticable {
Copy link
Owner

Choose a reason for hiding this comment

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

Hook probably should be Diagnosticable too.

Copy link
Contributor Author

@kranfix kranfix Aug 25, 2020

Choose a reason for hiding this comment

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

What's the need to make Hook diagnosticable?

StatelessWidget is Diagnosticable, but StatefulWidget isn't. Instead, State is Diagnosticable. In this case, Hook is like StatefulWidget and HookState is like State.

@rrousselGit
Copy link
Owner

Covered doesn't mean tested. We need a test that verifies that debugFillProperties works correctly.

A good start may be to have some existing hooks override debugFillProperties, like useState/useMemoized, and test that it works properly.

@kranfix
Copy link
Contributor Author

kranfix commented Aug 27, 2020

I don't find a way to test the debugFillProperties. Do you have any suggestion?

@kranfix
Copy link
Contributor Author

kranfix commented Aug 28, 2020

@rrousselGit I found a way to test de debugFillProperties . I'll be sending new commits on weekend.

@kranfix
Copy link
Contributor Author

kranfix commented Aug 31, 2020

Tests for useState and useMemoized where added

@rrousselGit
Copy link
Owner

Thanks, sorry for the delay.
I'll merge that and deploy it later this week. I need to make some extra changes

@rrousselGit
Copy link
Owner

rrousselGit commented Sep 5, 2020

I'll do that tomorrow.
I plan to add Diagnistics for all hooks at once.

And the Diagnostics must be updated such that we can have:

HookBuilder
 │ useMemoized<int>: 43
 | useAnimationController: AnimationController#00000
 | └duration: Duration(seconds: 2)
 └SizedBox(renderObject: RenderConstrainedBox#00000)

@rrousselGit rrousselGit merged commit 1bafe5c into rrousselGit:master Sep 6, 2020
@kranfix kranfix deleted the debug_fill_properties branch September 6, 2020 17:19
Phenek pushed a commit to Phenek/flutter_hooks that referenced this pull request Nov 30, 2025
HookWidget.debugFillProperties using HookState.diagnosticsProperty
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