Add widget inheritance links by HansMuller · Pull Request #6072 · flutter/flutter

HansMuller · 2016-09-26T17:58:35Z

Enable a widget to inherit from a widget that's not its parent.

The widget contained by an InheritedWidgetLinkChild inherits from the ancestors of the InheritedWidgetLinkParent that it is linked to.

The link itself is bi-directional and must be defined in terms of global keys. The value of the InheritedWidgetLinkParent's link is the InheritedWidgetLinkChild's global key, and vice versa.

The inheritance link is exlusive: the InheritedWidgetLinkChild only inherits from the InheritedWidgetLinkParent's ancestors, it no longer inherits from its own ancestors.

The value of the link may be null, which restores normal inherited widget behavior.

Fixes #5572

abarth · 2016-09-26T18:01:16Z

packages/flutter/lib/src/widgets/framework.dart

docs please

abarth · 2016-09-26T18:03:02Z

Can you test what happens if you create a cycle (e.g., the child above the parent)?

HansMuller · 2016-09-26T22:15:53Z

I've added asserts, docs, and more tests.

Hixie · 2016-09-26T22:25:45Z

packages/flutter/lib/src/widgets/framework.dart

Ideally these classes would be outside the framework.dart file. Is that possible?

Yes, I've moved them into widgets/inherited_link.dart

Hixie · 2016-09-26T22:26:50Z

packages/flutter/lib/src/widgets/framework.dart

GlobalKey key?

Yes and oops.

Hixie · 2016-09-26T22:26:51Z

packages/flutter/lib/src/widgets/framework.dart

Hixie · 2016-09-26T22:28:01Z

packages/flutter/lib/src/widgets/framework.dart

this would be redundant with the aforementioned change

Yes and done.

Hixie · 2016-09-26T22:28:32Z

packages/flutter/lib/src/widgets/framework.dart

use is! rather than !( is )

Hixie · 2016-09-26T22:29:23Z

packages/flutter/lib/src/widgets/framework.dart

similar comments here

Hixie · 2016-09-26T22:29:41Z

packages/flutter/lib/src/widgets/framework.dart

remove this if block

in fact in general you should see if this function needs updating; markNeedsBuild changed a bit recently

Hixie · 2016-09-26T22:31:12Z

packages/flutter/lib/src/widgets/framework.dart

wow this is quite expensive. is there no quicker way to do this?

Hixie · 2016-09-26T22:34:58Z

I suspect this doesn't work with LayoutBuilders. You should make sure you have tests where you have a layout builder and you change nothing (i.e. widgets are == across pumps) except that the layout builder is dirtied and something that would be inherited is dirtied, with situations like

      / ... -> layout builder -> ... -> inherited -> ... link parent 
root <
      \ ... -> link child -> ... -> inheritor

      / ... -> link child -> ... -> layout builder -> ... -> inheritor
root <
      \ ... -> inherited -> ... -> link parent

Note that in these cases, the ... bits are important. They must be widgets that do not change identity or get dirtied in any way across pumps.

Hixie · 2016-09-26T22:37:01Z

another couple of things to test:

      / link child -> inheritor
root <
      \ ... -> inherited -> ... -> link parent

      / link child -> inheritor
root <
      \ ... -> layout builder -> ... -> inherited -> ... link parent

...where the root and link child do get rebuilt (because you provide new, albeit unchanged, widgets in the second pump).

dragostis · 2016-09-27T21:57:02Z

packages/flutter/lib/src/material/popup_menu.dart

-    return new InkWell(
-      onTap: () { showButtonMenu(context); },
-      child: config.child
+


Here you could assert that button is not null.

dragostis · 2016-09-27T21:58:27Z

packages/flutter/lib/src/widgets/framework.dart

  }
 }

+/// An element that uses a [InheritedWidgetLinkParent] as its configuration.


'a' -> 'an'

dragostis · 2016-09-27T21:58:46Z

packages/flutter/lib/src/widgets/framework.dart

+  @override
+  InheritedWidgetLinkParent get widget => super.widget;
+
+  InheritedElementLinkChild get link => widget.link?._currentElement;


Does this need docs?

dragostis · 2016-09-27T22:00:19Z

packages/flutter/lib/src/widgets/framework.dart

+    final Element linkElement = widget.link?._currentElement;
+    if (linkElement != null && linkElement is! InheritedElementLinkChild) {
+      throw new FlutterError(
+        'InheritedWidgetLinkParent link value is not an InheritedWidgetLinkChild.\n'


Maybe we can print the runtime type in the error.

dragostis · 2016-09-27T22:01:36Z

packages/flutter/lib/src/widgets/framework.dart

+  @override
+  InheritedWidgetLinkChildProxy get widget => super.widget;
+
+  InheritedElementLinkParent get link => widget.link?._currentElement;


dragostis · 2016-09-27T22:03:31Z

packages/flutter/lib/src/widgets/framework.dart

+    final Element linkElement = widget.link?._currentElement;
+    if (linkElement != null && linkElement is! InheritedElementLinkParent) {
+      throw new FlutterError(
+        'InheritedWidgetLinkChild link value is not an InheritedWidgetLinkParent.\n'


Runtime type here as well.

HansMuller · 2016-09-28T19:06:52Z

The last version of this code passed all of the proposed tests (and then some) except:

      / link child -> inheritor
root <
      \ ... -> layout builder -> ... -> inherited -> ... link parent

In this case both halves of the link are built at layout time and the child is built first. That fails of course, because the child's parent and the values it's supposed to inherit aren't available. It's not possible to defer building the child until the parent has been built, since layout allocates sizes top-down.

The link child's implementation could report a zero size and notify the link parent that when it was laid out it should mark the child as needing to be rebuilt. This would cause the entire tree to be built again. Additional links could cause even more rebuilding which would kill performance.

The general purpose inheritance link is also flawed because in most cases one does not want the link child to inherit all of the link parent's inherited widgets. For example it would be a mistake to pick up the parent's TickerMode.

abarth reviewed Sep 26, 2016

View reviewed changes

packages/flutter/lib/src/widgets/framework.dart Outdated

Copy link

Contributor

abarth Sep 26, 2016

Choose a reason for hiding this comment

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

docs please

HansMuller force-pushed the inherited_widget_link branch from 274f4a4 to a577c02 Compare September 26, 2016 22:09

Hixie reviewed Sep 26, 2016

View reviewed changes

packages/flutter/lib/src/widgets/framework.dart Outdated

Copy link

Contributor

Hixie Sep 26, 2016

Choose a reason for hiding this comment

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

wow this is quite expensive. is there no quicker way to do this?

HansMuller force-pushed the inherited_widget_link branch from ad7c7ce to 5eaa842 Compare September 27, 2016 17:59

Hans Muller added 7 commits September 27, 2016 14:21

merge

3c085b1

use updateNotify() in link child

0c5b221

Additional assertions, tests, docs

3417b35

element doc one-liners

e6c8269

Simple updates per feedback

adb2661

Added cross-link test, various tweaks

d5c276c

Build link children at layout time

3801dc2

HansMuller force-pushed the inherited_widget_link branch from 5eaa842 to 3801dc2 Compare September 27, 2016 21:21

Added one setState driven LayoutBuilder test

c01d3e2

dragostis reviewed Sep 27, 2016

View reviewed changes

HansMuller closed this Sep 28, 2016

yjbanov mentioned this pull request Sep 19, 2017

dialog inherits Localizations #12172

Closed

This was referenced Jul 15, 2019

Widgets inserted to an Overlay have different ancestral InheritedWidget(s) than commonly expected #36220

Closed

Hero wrapping a widget that require Material widget ancestor #34119

Closed

github-actions bot locked as resolved and limited conversation to collaborators Aug 15, 2021

Conversation

HansMuller commented Sep 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

abarth commented Sep 26, 2016

Uh oh!

HansMuller commented Sep 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Hixie commented Sep 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Hixie commented Sep 26, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HansMuller commented Sep 28, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Hixie commented Sep 26, 2016 •

edited

Loading