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

ScrollView should account for Content margin #392

Merged
merged 2 commits into from Oct 25, 2016

Conversation

Projects
None yet
5 participants
@adrianknight89
Contributor

adrianknight89 commented Sep 29, 2016

Description of Change

ScrollView did not get the correct maximum height and width of its content and failed to show the correct content margin. If the content is 800 units tall with 200 units of vertical thickness, then _content.Bounds.Bottom returns 900 and Top is 100.

We shouldn't ignore top and left positions as well as top and left paddings.

Bugs Fixed

@dnfclas

This comment has been minimized.

Show comment
Hide comment
@dnfclas

dnfclas Sep 29, 2016

Hi @adrianknight89, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

dnfclas commented Sep 29, 2016

Hi @adrianknight89, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by .NET Foundation and real humans are currently evaluating your PR.

TTYL, DNFBOT;

@rmarinho rmarinho self-assigned this Oct 12, 2016

@rmarinho

Can you add a unittest ?

@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89
Contributor

adrianknight89 commented Oct 12, 2016

@rmarinho Done.

@rmarinho

This comment has been minimized.

Show comment
Hide comment
@rmarinho

rmarinho Oct 25, 2016

Member

@adrianknight89 do you mind rebase so i can run tests on this the windows certificates expired.

Member

rmarinho commented Oct 25, 2016

@adrianknight89 do you mind rebase so i can run tests on this the windows certificates expired.

@rmarinho rmarinho removed the review-needed label Oct 25, 2016

@adrianknight89

This comment has been minimized.

Show comment
Hide comment
@adrianknight89

adrianknight89 Oct 25, 2016

Contributor

@rmarinho Can you check now?

Contributor

adrianknight89 commented Oct 25, 2016

@rmarinho Can you check now?

@rmarinho rmarinho merged commit b4a46d4 into xamarin:master Oct 25, 2016

1 check passed

Windows-Manual-Debug Finished TeamCity Build Xamarin.Forms :: Debug :: Cycle8 :: Windows Manual PR Review : Tests passed: 3461, ignored: 8
Details

@samhouts samhouts added this to the 2.3.4 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment