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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 984983004: Correctly allow components to return different top-level nodes during different renderings (Closed)

Created:
5 years, 9 months ago by rafaelw
Modified:
5 years, 9 months ago
Reviewers:
abarth-chromium, Hixie
CC:
abarth-chromium, esprehn, 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

Correctly allow components to return different top-level nodes during different renderings TBR=abarth BUG= Committed: https://chromium.googlesource.com/external/mojo/+/ddb04a99eb257b043215a60fb65868afdb6e6462

Patch Set 1 #

Patch Set 2 : remove test file #

Total comments: 4

Patch Set 3 : cleanup #

Patch Set 4 : moar #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -1 line) Patch
M sky/framework/fn.dart View 1 2 3 1 chunk +7 lines, -1 line 0 comments Download

Messages

Total messages: 4 (1 generated)
rafaelw
Committed patchset #4 (id:60001) manually as ddb04a99eb257b043215a60fb65868afdb6e6462 (presubmit successful).
5 years, 9 months ago (2015-03-06 21:32:21 UTC) #2
Hixie
https://codereview.chromium.org/984983004/diff/20001/sky/framework/fn.dart File sky/framework/fn.dart (right): https://codereview.chromium.org/984983004/diff/20001/sky/framework/fn.dart#newcode618 sky/framework/fn.dart:618: // TODO(rafaelw): This eagerly removes the old VDOM. It ...
5 years, 9 months ago (2015-03-06 21:32:52 UTC) #3
rafaelw
5 years, 9 months ago (2015-03-06 21:37:29 UTC) #4
Message was sent while issue was closed. 
https://codereview.chromium.org/984983004/diff/20001/sky/framework/fn.dart
File sky/framework/fn.dart (right):

https://codereview.chromium.org/984983004/diff/20001/sky/framework/fn.dart#ne...
sky/framework/fn.dart:618: // TODO(rafaelw): This eagerly removes the old VDOM.
It may be that a
On 2015/03/06 21:32:52, Hixie wrote:
> Do you mean VDOM here, or DOM? _remove() seems to do both (though I can't
quite

You are correct, it is actually both (concretely, sky DOM nodes must be removed
and any components should get the didUnmount() signal).

> tell what it means to remove things from the VDOM, I thought that was an
> immutable structure).

Logically yes, though that's not how it's implemented.

https://codereview.chromium.org/984983004/diff/20001/sky/framework/fn.dart#ne...
sky/framework/fn.dart:620: // syncing the new VDROM against the old one.
On 2015/03/06 21:32:52, Hixie wrote:
> s/VDROM/VDOM/?

Yeah, I fixed that before landing.

Powered by Google App Engine
This is Rietveld 408576698