Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

[Android] Fix ScrollView visibility issue #10659

Merged
merged 2 commits into from
Sep 10, 2020
Merged

Conversation

jsuarezruiz
Copy link
Contributor

@jsuarezruiz jsuarezruiz commented May 12, 2020

Description of Change

In this PR #8090 was added the support to be able to manage the visibility of the scrollbar in the ScrollView on Android. However, this PR added a repeated issue, the scrollbar visibility the child elements of the ScrollView when initializing (or refreshing). For this reason, this PR #10447 revert original one changes.

This PR recovers the initial changes to allow managing the visibility of the scrollbar in ScrollView but also avoiding the issue with the child elements.

fixed-scrollview-visibility-droid

The problem was in the used Context. When creating ScrollViewRenderer we were creating a context that used a custom style (using vertical and horizontal scrollbars). This context was used creating ScrollViewContainer. So by default ScrollViewContainer had VerticalScrollBarEnabled set to true (incorrect!).

Issues Resolved

API Changes

None

Platforms Affected

  • Android

Behavioral/Visual Changes

None

Before/After Screenshots

Before

issue-scrollbar-scrollview

After

fixed-scroll-droid

Testing Procedure

Launch Core Gallery. Navigate to the "ScrollView Gallery" to verify that can hide/show the ScrollView toolbar. Also, navigate to the "Material Entry Gallery" (for example) and set the focus on different Entries to validate that the "flickering scrollbar" issue in the ScrollView children's is fixed.

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@jsuarezruiz jsuarezruiz changed the title Fixed ScrollView Visibility on Android [Android] Fix ScrollView visibility issue May 12, 2020
@samhouts
Copy link
Member

@jsuarezruiz Can you see if this PR fixes #10882, and if so, retarget to 4.6.0? Thanks!

@jsuarezruiz
Copy link
Contributor Author

jsuarezruiz commented Jun 2, 2020

After the E.Z changes in this PR #10893, I am going to refactor this one a little bit to apply the same changes.

@PureWeen PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Jun 2, 2020
@PureWeen PureWeen requested a review from samhouts June 2, 2020 16:53
@samhouts samhouts added 4.4.0 regression on 4.4.0 feedback-ticket Issue originates from https://developercommunity.visualstudio.com i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often i/regression m/high impact ⬛ labels Jun 2, 2020
@samhouts samhouts requested review from hartez and removed request for PureWeen June 11, 2020 16:52
@samhouts samhouts assigned hartez and unassigned PureWeen Jun 11, 2020
@hartez
Copy link
Contributor

hartez commented Jun 29, 2020

@jsuarezruiz It looks like this might also fix #7629?

@jsuarezruiz jsuarezruiz removed the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Jul 7, 2020
@jsuarezruiz
Copy link
Contributor Author

Applied changes based on the E.Z changes in #10893.

@samhouts samhouts added the approved Has two approvals, no pending reviews, and no changes requested label Jul 8, 2020
@samhouts
Copy link
Member

The failing tests on Android concern me. :(

@jsuarezruiz jsuarezruiz added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Jul 17, 2020
@jsuarezruiz
Copy link
Contributor Author

@samhouts Added the "DO-NOT-MERGE-!!!" label until review the failing tests.

@samhouts samhouts changed the base branch from 4.7.0 to 4.8.0 August 13, 2020 22:02
@samhouts samhouts added the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Aug 13, 2020
@samhouts samhouts added DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. and removed DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. labels Sep 1, 2020
@rmarinho
Copy link
Member

Failing test doesn't seem related

@rmarinho rmarinho merged commit 7483a99 into 4.8.0 Sep 10, 2020
@rmarinho rmarinho deleted the fix-droid-scrollvisibility branch September 10, 2020 12:05
@samhouts samhouts added this to the 4.8.0 milestone Sep 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4.4.0 regression on 4.4.0 a/scrollview approved Has two approvals, no pending reviews, and no changes requested ControlGallery DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. feedback-ticket Issue originates from https://developercommunity.visualstudio.com i/high Completely doesn't work, crashes, or is unusably slow, has no obvious workaround; occurs less often i/regression m/high impact ⬛ p/Android retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. t/bug 🐛
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants