Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Horizontal scrolling broken in 0.32.0-rc.0 #561

Closed
rozele opened this issue Aug 5, 2016 · 7 comments
Closed

Horizontal scrolling broken in 0.32.0-rc.0 #561

rozele opened this issue Aug 5, 2016 · 7 comments
Assignees

Comments

@rozele
Copy link
Collaborator

rozele commented Aug 5, 2016

Horizontal scrolling example is broken in UIExplorer.

@rozele rozele added this to the Core View Managers milestone Aug 5, 2016
@rozele rozele self-assigned this Aug 5, 2016
@rozele
Copy link
Collaborator Author

rozele commented Aug 5, 2016

Confirmed this is also an issue for 0.31.0.

@rozele
Copy link
Collaborator Author

rozele commented Aug 5, 2016

Turns out this is related to the update to Facebook.CSSLayout v2.0.1-pre. We may need to revert 104aa49 (cc @rigdern)

@rozele
Copy link
Collaborator Author

rozele commented Aug 5, 2016

@rigdern, it looks like if you call CalculateLayout() on the horizontal scrolling content, it works okay, but I'm not sure the right infrastructure for that. In the meantime, I'm going to revert 104aa49 and we can work on integrating it in a future release.

Probably there is something very simple that we can do to signal to the CSSLayout engine that the width measurement mode should be undefined for horizontal scrolls, thoughts?

@rigdern
Copy link
Contributor

rigdern commented Aug 5, 2016

@rozele I can take a look at this later today.

Just to double check, are you using the same UIExplorer code used in the iOS and Android versions of 0.31.0? After updating the layout engine, Facebook had to make some tweaks to the UIExplorer app to fix bugs due to differences in the layout engine implementation. For example, facebook/react-native#8248 was introduced and was fixed by facebook/react-native@dc5b490.

@rozele rozele closed this as completed Aug 5, 2016
@rozele rozele removed the in progress label Aug 5, 2016
@rigdern
Copy link
Contributor

rigdern commented Aug 5, 2016

@rozele What's the latest news on this? I saw that this bug got closed without a corresponding bug being opened for upgrading the layout engine. Should I still look into this or did you figure it out? If I need to look into it, are there instructions somewhere for running the UIExplorer app? Also, I'm still curious to hear an answer to the question in my earlier comment.

@rozele
Copy link
Collaborator Author

rozele commented Aug 5, 2016

I didn't open a new bug yet, but, yes, this still needs to be investigated. I think there's a README in the Examples folder for how to run the UIExplorer (or it may be the main README).

Sent from my Windows Phone


From: Adam Comellamailto:notifications@github.com
Sent: ‎8/‎5/‎2016 6:11 PM
To: ReactWindows/react-native-windowsmailto:react-native-windows@noreply.github.com
Cc: Eric Rozellmailto:erozell@outlook.com; Mentionmailto:mention@noreply.github.com
Subject: Re: [ReactWindows/react-native-windows] Horizontal scrolling broken in 0.32.0-rc.0 (#561)

@rozele What's the latest news on this? I saw that this bug got closed without a corresponding bug being opened for upgrading the layout engine. Should I still look into this or did you figure it out? If I need to look into it, are there instructions somewhere for running the UIExplorer app? Also, I'm still curious to hear an answer to the question in my earlier comment.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#561 (comment)

rigdern pushed a commit to rigdern/react-native-windows that referenced this issue Aug 6, 2016
A second attempt at updating the css-layout dependency. This commit
includes the changes from these 2 commits:
  - 104aa49: Update css-layout dependency
  - 4501096: Clean up warnings due to project.json

It also includes a fix for microsoft#561 (horizontal scrolling broken) which was
introduced by the previous attempt at upgrading the layout engine.
Facebook already found and fixed this bug in their repo so I imported
their fix: facebook/react-native@6603cef

The updated version of css-layout includes significant
changes which make the layout engine conform closer to
the W3C spec. For details, see facebook/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
Copy link
Contributor

rigdern commented Aug 6, 2016

I got UIExplorer running, diagnosed, and fixed the isse (PR #565).

The reason I was confused about how to run UIExplorer was I didn't see it in my local repo so I assumed it lived somewhere else. GitHub's file search also turned up 0 results for "UIExplorer". I didn't realize there was a git submodule I had to initialize to get UIExplorer.

rigdern pushed a commit to rigdern/react-native-windows that referenced this issue Aug 6, 2016
A second attempt at updating the css-layout dependency. This commit
includes the changes from these 2 commits:
  - 104aa49: Update css-layout dependency
  - 4501096: Clean up warnings due to project.json

It also includes a fix for microsoft#561 (horizontal scrolling broken) which was
introduced by the previous attempt at upgrading the layout engine.
Facebook already found and fixed this bug in their repo so I imported
their fix: facebook/react-native@6603cef

The updated version of css-layout includes significant
changes which make the layout engine conform closer to
the W3C spec. For details, see facebook/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 pushed a commit that referenced this issue Aug 7, 2016
A second attempt at updating the css-layout dependency. This commit
includes the changes from these 2 commits:
  - 104aa49: Update css-layout dependency
  - 4501096: Clean up warnings due to project.json

It also includes a fix for #561 (horizontal scrolling broken) which was
introduced by the previous attempt at upgrading the layout engine.
Facebook already found and fixed this bug in their repo so I imported
their fix: facebook/react-native@6603cef

The updated version of css-layout includes significant
changes which make the layout engine conform closer to
the W3C spec. For details, see facebook/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 pushed a commit that referenced this issue Aug 25, 2016
A second attempt at updating the css-layout dependency. This commit
includes the changes from these 2 commits:
  - 104aa49: Update css-layout dependency
  - 4501096: Clean up warnings due to project.json

It also includes a fix for #561 (horizontal scrolling broken) which was
introduced by the previous attempt at upgrading the layout engine.
Facebook already found and fixed this bug in their repo so I imported
their fix: facebook/react-native@6603cef

The updated version of css-layout includes significant
changes which make the layout engine conform closer to
the W3C spec. For details, see facebook/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.
GantMan pushed a commit to infinitered/react-native-windows that referenced this issue Sep 29, 2016
A second attempt at updating the css-layout dependency. This commit
includes the changes from these 2 commits:
  - 104aa49: Update css-layout dependency
  - 4501096: Clean up warnings due to project.json

It also includes a fix for microsoft#561 (horizontal scrolling broken) which was
introduced by the previous attempt at upgrading the layout engine.
Facebook already found and fixed this bug in their repo so I imported
their fix: facebook/react-native@6603cef

The updated version of css-layout includes significant
changes which make the layout engine conform closer to
the W3C spec. For details, see facebook/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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants