Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(4)

Issue 996103008: Sky/Effen refactor: all nodes receive _mount/_unmount signal (Closed)

Created:
5 years, 9 months ago by rafaelw
Modified:
5 years, 9 months ago
Reviewers:
abarth-chromium
CC:
abarth-chromium, mojo-reviews_chromium.org, ojan, qsr+mojo_chromium.org
Base URL:
https://github.com/domokit/mojo.git@master
Target Ref:
refs/heads/master
Project:
mojo
Visibility:
Public.

Description

Sky/Effen refactor: all nodes receive _mount/_unmount signal. This patch refactors Effen in advance of allowing a final transform step from Effen nodes to sky nodes. The central changes here are: 1) Effen nodes get a _parentNode pointer 2) The lifetime of effen nodes is ->_sync (0 to N times) -> the first _sync() causes a _mount() ->_unmount 3) Node should expect to sync even when they are first inserted (in which case they sync against a prototypical empty node which their class must provide). 4) Subclasses now override _syncNode() no longer takes host & insertBefore (_mount does). In the one case that a node must be replaced without being unmounted (inside _syncInternal), the old node is inspected for it's sky position. TBR=abarth Committed: https://chromium.googlesource.com/external/mojo/+/23cea7f0dfb1469d34afec2baa6d52ddb580e4da

Patch Set 1 #

Patch Set 2 : cleanup #

Patch Set 3 : moar #

Patch Set 4 : sync #

Patch Set 5 : a bit moar #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+143 lines, -120 lines) Patch
M sky/framework/fn.dart View 1 2 3 4 19 chunks +143 lines, -120 lines 4 comments Download

Messages

Total messages: 5 (1 generated)
rafaelw
5 years, 9 months ago (2015-03-17 03:00:14 UTC) #2
rafaelw
Committed patchset #5 (id:80001) manually as 23cea7f0dfb1469d34afec2baa6d52ddb580e4da (presubmit successful).
5 years, 9 months ago (2015-03-17 03:01:21 UTC) #3
abarth-chromium
lgtm https://codereview.chromium.org/996103008/diff/80001/sky/framework/fn.dart File sky/framework/fn.dart (right): https://codereview.chromium.org/996103008/diff/80001/sky/framework/fn.dart#newcode152 sky/framework/fn.dart:152: static Text _emptyText = new Text(null); final https://codereview.chromium.org/996103008/diff/80001/sky/framework/fn.dart#newcode284 ...
5 years, 9 months ago (2015-03-17 03:23:40 UTC) #4
rafaelw
5 years, 9 months ago (2015-03-18 19:08:32 UTC) #5
Message was sent while issue was closed.
https://codereview.chromium.org/996103008/diff/80001/sky/framework/fn.dart
File sky/framework/fn.dart (right):

https://codereview.chromium.org/996103008/diff/80001/sky/framework/fn.dart#ne...
sky/framework/fn.dart:152: static Text _emptyText = new Text(null);
On 2015/03/17 03:23:40, abarth wrote:
> final

fixed in https://codereview.chromium.org/1009543008

https://codereview.chromium.org/996103008/diff/80001/sky/framework/fn.dart#ne...
sky/framework/fn.dart:284: // print("---Syncing children of $_key");
On 2015/03/17 03:23:40, abarth wrote:
> Do we want to leave these prints in?

fixed in https://codereview.chromium.org/1009543008

Powered by Google App Engine
This is Rietveld 408576698