fix(android): widget multi-target click listeners for reliable deep linking#572
fix(android): widget multi-target click listeners for reliable deep linking#572drehelis wants to merge 4 commits intobetter-rail:mainfrom
Conversation
Summary of ChangesHello @drehelis, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the reliability of deep linking from Android widgets by implementing a multi-target click handling mechanism. By allowing multiple views within a widget to register for click events, it resolves previous issues where clicks were frequently missed, especially on the 2x2 widget layout. This change ensures a more consistent and user-friendly experience when interacting with widgets. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of unreliable widget clicks by implementing multi-target click listeners. The changes are well-structured, introducing a new getClickTargetIds() function and correctly propagating its use throughout the widget providers. My review includes a couple of suggestions for improvement: one to refactor a section for better conciseness and efficiency, and another to align with Kotlin best practices by preferring List over Array to enhance code safety.
| val actionRefresh: String, | ||
| val actionWidgetUpdate: String, | ||
| val actionRouteReversal: String, | ||
| val clickTargets: IntArray |
There was a problem hiding this comment.
For better safety and adherence to Kotlin best practices, consider using List<Int> instead of IntArray. IntArray uses reference equality for equals() and hashCode(), which can lead to subtle bugs. List<Int> uses structural equality, which is usually what's intended.
This change would require updating getClickTargetIds() to return List<Int>, setupClickIntentsBase() to accept a List<Int>, and using listOf() instead of intArrayOf().
| val clickTargets: IntArray | |
| val clickTargets: List<Int> |
guytepper
left a comment
guytepper
left a comment
There was a problem hiding this comment.
Note
This review was generated by Claude Code.
Overall
Good, focused fix. The multi-target click approach is the right solution — registering the same PendingIntent on multiple view layers (root container, content overlay, station background) ensures touch events are captured regardless of which layer intercepts them. The refactoring from single clickTargetId to clickTargetIds: IntArray is clean and backward-compatible.
Potential Issue
setupConfigurationClickIntent (line 712) was not updated. It still calls views.setOnClickPendingIntent(getWidgetContainerId(), pendingIntent) with a single target. Since containerId now points to the content overlay (widget_content_container_2x2/widget_content_container_4x2) instead of the root, the "Tap to configure" state may suffer from the same click-not-registering issue on the background/root layers — the very bug this PR is fixing for the schedule state.
Suggest updating setupConfigurationClickIntent to iterate over getClickTargetIds() as well, for consistency.
| clickTargets = intArrayOf( | ||
| R.id.widget_container_compact, | ||
| R.id.widget_content_container_2x2, | ||
| R.id.widget_station_background | ||
| ), |
There was a problem hiding this comment.
[Claude Review]
Good coverage — targeting the root container, content overlay, and station background should handle all the touch interception layers.
Minor Kotlin note: IntArray uses reference equality, not structural equality. This won't matter for enum constants (always the same instance), but if clickTargets is ever compared in tests or other contexts, == won't behave like contentEquals(). Not actionable now, just a heads-up.
2 participants
Fixes an issue where widget clicks were often ignored on the 2x2 layout due to touch events being consumed by the background overlay or not bubbling up to the root container.