Skip to content

Update css-layout dependency#547

Closed
rigdern wants to merge 1 commit into
microsoft:masterfrom
rigdern:rigdern/upgradeLayout
Closed

Update css-layout dependency#547
rigdern wants to merge 1 commit into
microsoft:masterfrom
rigdern:rigdern/upgradeLayout

Conversation

@rigdern

@rigdern rigdern commented Jul 30, 2016

Copy link
Copy Markdown
Contributor

The updated version of css-layout includes significant
changes which make the layout engine conform closer to
the W3C spec. For details, see react/yoga#185

The inline view implementation had to be modified slightly
due to a change in the layout engine. In the updated layout
engine, nodes with a measure function are treated as leaves.
Consequently, nodes with a mesaure function (e.g. Text) do
not have their children laid out automatically.

To fix this, Text nodes now manually invoke the layout engine
on each of their inline views.

@rozele

rozele commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

What if we created a wrapper view manager that returned the InlineUIElement type and just added logic to it's ReactShadowNode implementation to call CalculateLayout in the OnBeforeLayout hook?

@rozele

rozele commented Aug 1, 2016

Copy link
Copy Markdown
Contributor

Or, in theory, you could just put the logic to call child.CalculateLayout in the OnBeforeLayout hook for ReactSpanShadowNode and ReactTextShadowNode. The current proposal seems like a very non-explicit way of ensuring unreachable CSSNodes are have CalculateLayout invoked recursively.

I still prefer the first suggestion, but I'm not sure exactly the best way to inject such behavior into Text.windows.js.

@rigdern rigdern force-pushed the rigdern/upgradeLayout branch from efbc242 to 4ca7d21 Compare August 2, 2016 02:46
The updated version of css-layout includes significant
changes which make the layout engine conform closer to
the W3C spec. For details, see react/yoga#185

The inline view implementation had to be modified slightly
due to a change in the layout engine. In the updated layout
engine, nodes with a measure function are treated as leaves.
Consequently, nodes with a mesaure function (e.g. Text) do
not have their children laid out automatically.

To fix this, Text nodes now manually invoke the layout engine
on each of their inline views.
@rigdern rigdern force-pushed the rigdern/upgradeLayout branch from 4ca7d21 to c47ef7b Compare August 2, 2016 02:48
@rigdern

rigdern commented Aug 2, 2016

Copy link
Copy Markdown
Contributor Author

@rozele I implemented the second suggestion.

I don't have a preference between the first and second suggestions but for the first one, I'm not sure how you would wrap each inline view with a special view manager. If you were thinking of having the Text component's render function enumerate this.props.children and wrap each inline view with a special view, I think we'll run into a problem. The problem is this.props.children contains a mixture of native primitives (e.g. View, Text, ScrollView) and components (subclasses of React.Component). When you encounter a React.Component, you want to know what native primitives it'll render down into. I don't know of any easy way of getting at this information in JavaScript.

@rozele

rozele commented Aug 2, 2016

Copy link
Copy Markdown
Contributor

Fixed merge conflict and pushed directly to master.

@rozele rozele closed this Aug 2, 2016
@rigdern rigdern deleted the rigdern/upgradeLayout branch August 4, 2016 19:22
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