Fixes Issue #6384: java.lang.NullPointerException in ReviewActivity #6394
Conversation
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
Before this commit, the info icon shared the same GUI element with the "Skip this image" text. This made the Kotlin code to handle taps on the info icon difficult to write, and would crash with a NPE when the user used a language that is read right to left and the info icon was pressed. This commit creates new GUI elements. Notably, the info icon has it's own element. A LinearLayout is used to place the skip button and the info icon button together. Kotlin code can now be simplified and the NPE bug can be fixed.
Before this commit, if the language was set to a language that is read right to left, pressing the info icon would crash the app with a NPE. This was because the Kotlin code assumed that the icon would always be on the right of the skip button (index 2 in the drawable array). When a right to left language was used, the icon would be on the left and index 2 would be null. This commit builds upon prior GUI changes. The info icon now has its own button. Kotlin changes now remove the use of the drawable array to find the info icon and instead directly references the new info icon button. The info icon button now works properly for both left-to-right and right-to-left languages while maintaining correct positioning.
This commit moves around some lines in the XML to make it more readable.
|
The "Skip this image" text and the info icon appear to have more space between them in this PR with the way I wrote the XML. I thought about moving them closer to their original distance, but I think it might be better if they were further apart. If the two buttons are too close, then users may accidentally press the wrong one, especially on smaller screens. If needed, I can easily change the distance between the buttons. |
|
✅ Generated APK variants! |
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.
Description (required)
Fixes #6384
What changes did you make and why?
The GUI for the Review Activity was modified so that the info icon (the "I" inside the circle), had it's own button rather than being a
Drawablefor the "Skip this image" button.The Kotlin code was then simplified to show the information when this new button was pressed.
These changes eliminate the NPE because the Kotlin code no longer uses array accesses to find the icon and instead refers directly to the new button.
Tests performed (required)
How to test this fix:
On main, the app will crash with a NPE. On this branch, the popup appears correctly.
The same steps can be repeated with a left to right language (e.g. English) to confirm that the info icon appears on the right and is working correctly.
Tested ProdDebug on Android Studio emulator with API level 36.
Screenshots (for UI changes only)
