Repository navigation
Conversation
The sort button and the playlist action buttons were supplied to the adapter through getListHeaderSupplier(), so they were rendered as list item 0 and scrolled out of the screen on long history lists. Pin them above the list instead, in a new fragment_history layout which reuses fragment_playlist for the list, the loading indicator, the empty state and the error panel. Bind the controls once in initViews() and register the sort listener in initListeners() instead of re-registering it on every database emission. The controls are hidden while the history is empty or an error is shown, but no longer while a reload is running: hiding them in showLoading() made the whole bar blink and the list jump every time the sort mode was toggled.
The first version scrolled the RecyclerView from the instrumentation thread and sized the screen in raw pixels, so the assertions depended on the density of the emulator and on the test running off the main thread. Run the whole test body on the main thread, express the screen size in dp, and add assertions for the controls not covering the list and for the controls not eating up a landscape screen.
github-actions
Bot
added
size/medium
Contributor
|
Please link this PR to an existing issue you are assigned to. This PR might be closed if the details of this PR were not agreed upon beforehand. |
|
Author
|
Closing this: it was opened to document the work for a course assignment, and I had noticed too late that the issue is already being handled by @kenneySiu. Sorry for the noise, no merge was expected. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is it?
Description of the changes in your PR
The sort button and the playlist action buttons of the history screen were handed to the adapter through
getListHeaderSupplier(), so they were rendered as list item 0 and scrolled out of the screen as soon as the history list became longer than one screen.fragment_history.xml, which pinsstatistic_playlist_controlabove a reusedfragment_playlistlayout, so the list, the loading indicator, the empty state and the error panel keep working unchangedStatisticsPlaylistFragmentnow binds the controls that are already in the layout instead of supplying a list headerinitListeners()instead of being re-registered on every database emissionNote: I noticed after starting that this issue is already assigned to @kenneySiu. I am submitting this PR so that the work is documented for a university course assignment (COMP6120 at ANU). I would be happy to close it if you would rather handle this differently - no merge is expected.
Before/After Screenshots/Screen Record
Fixes the following issue(s)
Relies on the following changes
Testing/QA
Devices
HistoryControlsLayoutTestinstrumentation test; the CI runsconnectedCheckon API 23 and API 35checkstyleandktlintare clean on the changed filesSteps for testing