Avoid showing vertical scroll bar on stage error listview #1097

Merged
kekkyojin merged 1 commit from remove_stages_scrollbar into master 2020-12-29 07:00:03 +01:00
kekkyojin commented 2020-12-22 02:39:29 +01:00 (Migrated from github.com)

PR Checklist

Please check all that apply to this PR using "x":

  • I have checked that this PR is not a duplicate of an existing PR (open, closed or merged)
  • I have checked that this PR does not introduce a breaking change

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

On some cases it could happen that the list which shows startup stage errors is also showing a scroll bar, which also makes not all stages be seen at the same time

What is the new behavior?

Listview is measured and resized to a height which would allow all stages statuses to be seen at the same time

Other information

Testing on the emulators didn't show the current behavior. However, testing on my physical device did show it. I suggest testing this PR on a physical device.

ListView layout_height on app_bar_main.xml was changed to "0dp" as Android Studio was recommending that change.

## PR Checklist <!-- For the checkbox formatting to work properly, make sure there are no spaces on either side of the "x" --> Please check all that apply to this PR using "x": - [x] I have checked that this PR is not a duplicate of an existing PR (open, closed or merged) - [x] I have checked that this PR does not introduce a breaking change ## PR Type What kind of change does this PR introduce? - [x] Bugfix ## What is the current behavior? On some cases it could happen that the list which shows startup stage errors is also showing a scroll bar, which also makes not all stages be seen at the same time ## What is the new behavior? Listview is measured and resized to a height which would allow all stages statuses to be seen at the same time ## Other information Testing on the emulators didn't show the current behavior. However, testing on my physical device did show it. I suggest testing this PR on a physical device. ListView layout_height on app_bar_main.xml was changed to "0dp" as Android Studio was recommending that change. <!-- If this PR contains a breaking change, please describe the impact and solution strategy for existing applications below. -->
akinwale (Migrated from github.com) reviewed 2020-12-29 06:56:03 +01:00
akinwale (Migrated from github.com) left a comment

Hey, thanks for this. Wouldn't it just be easier to set android:scrollbars="none" on the ListView in the layout xml? Is there a particular reason for taking this approach?

Edit: I actually, I see the reason now. I wasn't getting the behaviour with some being cut off when I tested, so maybe it also depends on device screen resolution.

Hey, thanks for this. Wouldn't it just be easier to set `android:scrollbars="none"` on the `ListView` in the layout xml? Is there a particular reason for taking this approach? Edit: I actually, I see the reason now. I wasn't getting the behaviour with some being cut off when I tested, so maybe it also depends on device screen resolution.
akinwale (Migrated from github.com) approved these changes 2020-12-29 06:59:51 +01:00
akinwale (Migrated from github.com) left a comment

Looks good to me.

Looks good to me.
kekkyojin commented 2020-12-29 10:25:09 +01:00 (Migrated from github.com)

Hey, thanks for this. Wouldn't it just be easier to set android:scrollbars="none" on the ListView in the layout xml? Is there a particular reason for taking this approach?

Edit: I actually, I see the reason now. I wasn't getting the behaviour with some being cut off when I tested, so maybe it also depends on device screen resolution.

Exactly. Testing on the emulator with any Pixel AVD didn't show the problem at all. It was on my physical Redmi device that list only showed a few items. Setting scrollbars='none' only hide the scrollbars, letting the list still not at its full height. That attribute is now not needed, but I don't see it as doing anything wrong neither.

> Hey, thanks for this. Wouldn't it just be easier to set `android:scrollbars="none"` on the `ListView` in the layout xml? Is there a particular reason for taking this approach? > > Edit: I actually, I see the reason now. I wasn't getting the behaviour with some being cut off when I tested, so maybe it also depends on device screen resolution. Exactly. Testing on the emulator with any Pixel AVD didn't show the problem at all. It was on my physical Redmi device that list only showed a few items. Setting `scrollbars='none'` only hide the scrollbars, letting the list still not at its full height. That attribute is now not needed, but I don't see it as doing anything wrong neither.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-android#1097
No description provided.