Starred issues by scottpeterson · Pull Request #296 · 5calls/android

scottpeterson · 2026-03-16T20:42:14Z

What type of PR is this? (check all applicable)

Refactor
Feature
Bug Fix
Optimization

Description

Added a Bookmarking feature to the app.

Issues can be bookmarked/unbookmarked from the List view or the Details view.

There is a db schema change.

The way a user's Bookmarked Issues are accessed is via the Top Issues filter.

There is analytics tracking via Plausible.

Related Issues

Related Issue #
Closes #

Were the changes tested?

Yes, automated tests in please name test methods or files
Yes, manually tested: please provide steps performed to test changes
No, and this is why: please replace this line with details on why tests
have not been included
I need help with writing tests

Users can bookmark individual issues This requires a database schema change!

dektar

This is great! I haven't had time yet to try it on a device, and I'll do that before I approve to check edge cases manually (I'm more likely to catch something that way than reading the code). I also haven't fully read the IssuesAdapterTest.

I think we could add to MainActivityHappyPathTest to try adding/removing a bookmark, or add a TODO for that.
You'll need Spanish strings before the changes pass the checks. I've been using Google Translate since we don't have a large Spanish-language presence at this point...
Sorry about the merge conflict, please make sure not to show a bookmark icon on the demo issue (we could do like if isPlaceholder { bookmarkIcon.setVisiblity(View.GONE) } or something in issuesAdapter.
Some minor comments below.

5calls/app/src/main/java/org/a5calls/android/a5calls/adapter/IssuesAdapter.java

5calls/app/src/main/java/org/a5calls/android/a5calls/controller/MainActivity.java

5calls/app/src/main/res/layout/activity_issue.xml

5calls/app/src/main/res/values-es/strings.xml

5calls/app/src/test/java/org/a5calls/android/a5calls/model/DatabaseHelperTest.java

Adding in spanish translations

* origin/master: Add a demonstration issue shown until the user has made 2 calls (#295)

on details view as well

dektar · 2026-03-18T03:14:27Z

Some comments from trying it on Pixel XL running Android 10 (SDK 29) and Pixel 9 emulator running SDK 36. It looks like a lot, but there's only a few functional issues that I think are big. If you want help or more details about these let me know.

Functional

Blocking launch: When I open the filter list using TalkBack, TalkBack's focus doesn't jump into the list. In fact, I can't get to the list at all using TalkBack's linear navigation. Similarly, when I open it with keyboard the keyboard focus isn't moved into the list. The filter list needs to get accessibility (and keyboard) focus when it opens up.
Blocking launch: When I bookmark an issue in the list using TalkBack, accessibility focus jumps to a different bookmark icon. Focus shouldn't jump when taking an inline action like this.
Blocking launch: The filter needs to have "Top issues" selected by default, not "all issues".
Because the filter drop-down is now shown below the filter red bar, I'm able to tap on the red bar while the drop-down is open. This quickly closes and re-opens the filter rather than just closing it. This was confusing -- can a click on it when it's already open just close it?
When I start to scroll on the filter drop-down list, the whole list view moves up/down as I scroll. This is minor but a bit weird.

Layout

The UI looks strange before a location is set. What about not showing bookmarks yet in this case? Otherwise we'd have to shift the layout -- there's too much whitespace.

The vertical height still seems a little tall when there's no previous calls, compared to before this change. Can we shrink the bottom padding of the row so the button ends up closer to the edges?
.
Can we line up the right edge of the bookmarks bar with the right edge of the state label?

The bookmark icon is a little smaller perhaps on the IssueActivity? Can it be made the same size as in the MainActivity? Also, super minor, but it seems to not be centered on this one-line issue title, and it's mostly centered on the multi-line issue title. What if it's always in the top right corner no matter the issue title length, next to or below the (sometimes visible) state name? It seems kind of out of place when it floats against such a long issue title.

Fixed accessibility issues (talkback) Moved top issues to top in Filter list, so index 0 as the default selection just works as before fixed the filter re-opening on filter bar tap issue fixed scroll issue of filter list partially over filter bar bookmark icon hidden when no location set reduced bottom whitespace since bookmark icon provides some additional bottom padding bookmark icon size is identical between MainActivity and IssueActivity bookmark icon on IssueActivity locked to Top intead of centered vertically on Issue title

scottpeterson · 2026-03-24T15:31:34Z

Should have fixed all comments. Commit pushed! @dektar

dektar

I will try it on the phone tomorrow, sorry I've run out of time today! Just a few things.

dektar · 2026-03-25T04:19:28Z

5calls/app/src/main/java/org/a5calls/android/a5calls/adapter/IssuesAdapter.java

+            // so it still reserves space for consistent row height.
            if (issue.isPlaceholder) {
-                vh.bookmarkIcon.setVisibility(View.GONE);
+                vh.bookmarkIcon.setVisibility(View.INVISIBLE);


do we also need to get rid of the content description or is this sufficient to hide for a11y? (i can test this later).

dektar · 2026-03-25T04:21:05Z

5calls/app/src/main/java/org/a5calls/android/a5calls/adapter/IssuesAdapter.java

            if (issue.isPlaceholder) {
-                vh.bookmarkIcon.setVisibility(View.GONE);
+                vh.bookmarkIcon.setVisibility(View.INVISIBLE);
+                vh.bookmarkIcon.setClickable(false);


do we ever set it to clickable(true)? if this view gets recycled it'll still have an unclickable icon?

dektar · 2026-03-25T04:21:34Z

5calls/app/src/main/java/org/a5calls/android/a5calls/controller/MainActivity.java

        return maxWidthPx;
    }

+


nit: extra newlines

dektar · 2026-03-25T04:21:49Z

5calls/app/src/main/res/layout/issue_view.xml

-        android:layout_width="54dp"
-        android:layout_height="54dp"
-        android:padding="12dp"
+        android:layout_width="48dp"


please prefer using constants for dimens rather than hard-coding.

scottpeterson added 2 commits March 14, 2026 14:41

New Bookmark / Saved Issues feature!

757e229

Users can bookmark individual issues This requires a database schema change!

Collapsed Bookmarks toggle into Top Issues filter

f91000b

scottpeterson requested a review from dektar March 16, 2026 20:42

scottpeterson self-assigned this Mar 16, 2026

dektar requested changes Mar 17, 2026

View reviewed changes

scottpeterson added 3 commits March 17, 2026 08:58

Fixing PR comments

474c771

Adding in spanish translations

Merge remote-tracking branch 'origin/master' into starred-issues

f2874f9

* origin/master: Add a demonstration issue shown until the user has made 2 calls (#295)

Had to hide bookmark for starter issue

abc2e96

on details view as well

fixing build error in test

b9bfbaf

dektar reviewed Mar 25, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Starred issues#296

Starred issues#296
scottpeterson wants to merge 7 commits intomasterfrom
starred-issues

scottpeterson commented Mar 16, 2026 •

edited

Loading

Uh oh!

dektar left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dektar commented Mar 18, 2026 •

edited

Loading

Uh oh!

scottpeterson commented Mar 24, 2026

Uh oh!

dektar left a comment

Uh oh!

dektar Mar 25, 2026

Uh oh!

dektar Mar 25, 2026

Uh oh!

dektar Mar 25, 2026

Uh oh!

dektar Mar 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

scottpeterson commented Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this? (check all applicable)

Description

Related Issues

Were the changes tested?

Uh oh!

dektar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dektar commented Mar 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Functional

Layout

Uh oh!

scottpeterson commented Mar 24, 2026

Uh oh!

dektar left a comment

Choose a reason for hiding this comment

Uh oh!

dektar Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

dektar Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

dektar Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

dektar Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

scottpeterson commented Mar 16, 2026 •

edited

Loading

dektar commented Mar 18, 2026 •

edited

Loading