[ASDisplayNode] Add automatic measure before layout #1673
Conversation
maicki
changed the title
ghost
added
the
| [self.view addSubnode:_rootNode]; | ||
| } | ||
|
|
||
| - (void)viewDidLayoutSubviews |
There was a problem hiding this comment.
Being able to delete this is really fantastic!
However, instead of setting the frame in the appearance method, should we instead set it in the viewWillLayoutSubviews method? This may be an important distinction for rotation.
There was a problem hiding this comment.
It should actually not be in viewWillAppear: at all. As this get's called every time the view controller appears. I moved the code to viewDidLoad.
To your question. As you can see while creating the root node we set the autoresizing mask to flexible width and height so we don't have to adjust the frame after a layout of the view controller subviews. You can also see it if you set a background color to the root node it expands to the whole view also if rotation is happening.
|
@maicki great change! I'm very excited about this; it's important behavioral polish. I'm ready to land after a few small updates. |
…oes not Normally measure will be called before layout occurs. If this doesn't happen, nothing is going to call it at all. An experimenting developer probably added a node to the hierarchy directly. We simply call measureWithSizeRange: using a size range equal to whatever bounds were provided to that element. This make initial experimentation using layoutSpecs much easier. Furthermore added logging if no size is given for the node before layout occurs.
|
@appleguy Addressed all comments and added some comments for things I didn't address in code |
| // Normally measure will be called before layout occurs. If this doesn't happen, nothing is going to call it at all. | ||
| // We simply call measureWithSizeRange: using a size range equal to whatever bounds were provided to that element or | ||
| // try to measure the node with the largest size as possible | ||
| if (self.supernode == nil && !self.supportsRangeManagedInterfaceState && !_flags.isMeasured) { |
There was a problem hiding this comment.
I don't think we should check that there is no parent. It is very possible in practice for an element to be included in manual layout, but not be present in any layout specification for its parent. @maicki it is possible I am overlooking something that you have considered, feel free to let me know if you think we should keep that part of the condition.
I actually have exactly the same question about the range managed check. Assuming an element is excluded from the layout specification, perhaps accidentally. It would be better for something to ultimately trigger a measure pass even if it appears in the upper left of the view by default. This could help developers understand that they forgot to position the view with the layout spec
|
I am jetlagged and pretty tired, I think I may have misunderstood the dynamic here, in terms of whether or not a a inclusion in a layout specification is relevant for this case. There may not be any changes necessary, but I would love your input on them just to make sure. No action or response needed if you're confident that no changes are advisable. |
Revert "Merge pull request #1673 from maicki/AddAutomaticMeasureBeforeLayout"
… Style Changes (facebookarchive#1674) * This PR addresses an issue I raised with facebookarchive#1673 where the clipping corner layer doesn't get re-rendered when going from a dark to light user interface style. The fix is to add code in the `-asyncTraitCollectionDidChangeWithPreviousTraitCollection:` method that calls through to `-_updateClipCornerLayerContentsWithRadius:backgroundColor:` if the user interface style changes. I've also added snapshot tests to verify that using clipping corners with a dynamic background color updates when the user interface style changes.
2 participants
Normally
measure:will be called on aASDisplayNodebefore layout occurs. If this doesn't happen, nothing is going to call it at all. An experimenting developer probably added a node to the hierarchy directly. We now simply callmeasureWithSizeRange:using a size range equal to whatever bounds were provided to that element. This will make initial experimentation using layoutSpecs much easier.