Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 21 additions & 22 deletions app/src/main/java/protect/card_locker/LoyaltyCardViewActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -815,8 +815,6 @@ public boolean onCreateOptionsMenu(Menu menu) {

@Override
public boolean onPrepareOptionsMenu(Menu menu) {
super.onPrepareOptionsMenu(menu);

if (loyaltyCard != null) {
// Update star status
if (loyaltyCard.starStatus == 1) {
Expand All @@ -829,15 +827,21 @@ public boolean onPrepareOptionsMenu(Menu menu) {

// Update archive/unarchive button
if (loyaltyCard.archiveStatus != 0) {
menu.findItem(R.id.action_unarchive).setVisible(true);
menu.findItem(R.id.action_archive).setVisible(false);
menu.findItem(R.id.action_archive_unarchive).setTitle(R.string.unarchive);
menu.findItem(R.id.action_archive_unarchive).setIcon(R.drawable.ic_unarchive);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
menu.findItem(R.id.action_archive_unarchive).setTooltipText(getString(R.string.unarchive));
}
Comment on lines +833 to +835
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What difference does this setTooltip call make? I'm not seeing any difference between this and the favourite button when I press or long-press it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It adds a small tooltip that appears when you long-press or hover over the icon (on Android 8.0+). It’s mainly for accessibility and clarity — helps users understand what the icon does. The difference is subtle, so it’s easy to miss unless you long-press for a moment.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's the point I'm trying to make: the long-press behaviour is exactly the same for this and the favourite button but the favourite button doesn't have this function call (on Android 15 at least):

image image

For hover, I'm not sure how to test that, any info?

} else {
menu.findItem(R.id.action_unarchive).setVisible(false);
menu.findItem(R.id.action_archive).setVisible(true);
menu.findItem(R.id.action_archive_unarchive).setTitle(R.string.archive);
menu.findItem(R.id.action_archive_unarchive).setIcon(R.drawable.ic_outline_archive);
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
menu.findItem(R.id.action_archive_unarchive).setTooltipText(getString(R.string.archive));
}
}
}

return true;
return super.onPrepareOptionsMenu(menu);
Comment thread
vijay2909 marked this conversation as resolved.
Outdated
}

@Override
Expand Down Expand Up @@ -874,22 +878,17 @@ public boolean onOptionsItemSelected(MenuItem item) {
invalidateOptionsMenu();

return true;
} else if (id == R.id.action_archive) {
DBHelper.updateLoyaltyCardArchiveStatus(database, loyaltyCardId, 1);
Toast.makeText(LoyaltyCardViewActivity.this, R.string.archived, Toast.LENGTH_LONG).show();

ShortcutHelper.removeShortcut(LoyaltyCardViewActivity.this, loyaltyCardId);
new ListWidget().updateAll(LoyaltyCardViewActivity.this);

// Re-init loyaltyCard with new data from DB
onResume();
invalidateOptionsMenu();

return true;
} else if (id == R.id.action_unarchive) {
DBHelper.updateLoyaltyCardArchiveStatus(database, loyaltyCardId, 0);
Toast.makeText(LoyaltyCardViewActivity.this, R.string.unarchived, Toast.LENGTH_LONG).show();
} else if (id == R.id.action_archive_unarchive) {
if(loyaltyCard.archiveStatus == 0){
DBHelper.updateLoyaltyCardArchiveStatus(database, loyaltyCardId, 1);
Toast.makeText(LoyaltyCardViewActivity.this, R.string.archived, Toast.LENGTH_LONG).show();

ShortcutHelper.removeShortcut(LoyaltyCardViewActivity.this, loyaltyCardId);
new ListWidget().updateAll(LoyaltyCardViewActivity.this);
}else{
DBHelper.updateLoyaltyCardArchiveStatus(database, loyaltyCardId, 0);
Toast.makeText(LoyaltyCardViewActivity.this, R.string.unarchived, Toast.LENGTH_LONG).show();
}
Comment thread
vijay2909 marked this conversation as resolved.
Outdated
// Re-init loyaltyCard with new data from DB
onResume();
invalidateOptionsMenu();
Expand Down
5 changes: 5 additions & 0 deletions app/src/main/res/drawable/ic_outline_archive.xml
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This icon seems to use a completely different arrow than the other archive icons. Can we maybe make this icon consistent with the other 2?

(Seems this one is from "Material Symbols Outlined", while the others are from "Outlined" and "Filled", so using "archive" from the "Outlined" set should fix this)

This icon (ic_outline_archive.xml) loyalty_card_icon_archived.xml ic_unarchive.xml
image image image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I think it might also be better to use "loyalty_card_icon_archived.xml" for when the icon is archived, as that is more consistent with the favourite icon which shows the current state as opposed to the action pressing the button will do, but I'm honestly not sure what the official Google design language says should be used)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Originally, the Archive icon represented the action (like Gmail), but Gmail removes the item from the list after archiving. Since we’re keeping the card visible in the list, it makes more sense to show the icon as a current state indicator rather than an action.
Star icons are similar — they show state. So now, Unarchive shows if the card is unarchived, Archive if it’s archived, making the status clear at a glance.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But now the star and archive buttons have completely opposite semantics, despite being next to each other. Star shows the current state, archive shows what state change will happen when you press it. That seems very confusing to me, which is why I suggested making both show the current state for consistency, using the non-outline vs. outline differences to show the difference in if it's archived or not (like how the star for favouriting currently works)

Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android" android:height="24dp" android:tint="?attr/colorControlNormal" android:viewportHeight="960" android:viewportWidth="960" android:width="24dp">

<path android:fillColor="@android:color/white" android:pathData="M480,720L640,560L584,504L520,568L520,400L440,400L440,568L376,504L320,560L480,720ZM200,320L200,760Q200,760 200,760Q200,760 200,760L760,760Q760,760 760,760Q760,760 760,760L760,320L200,320ZM200,840Q167,840 143.5,816.5Q120,793 120,760L120,261Q120,247 124.5,234Q129,221 138,210L188,149Q199,135 215.5,127.5Q232,120 250,120L710,120Q728,120 744.5,127.5Q761,135 772,149L822,210Q831,221 835.5,234Q840,247 840,261L840,760Q840,793 816.5,816.5Q793,840 760,840L200,840ZM216,240L744,240L710,200Q710,200 710,200Q710,200 710,200L250,200Q250,200 250,200Q250,200 250,200L216,240ZM480,540L480,540L480,540Q480,540 480,540Q480,540 480,540L480,540Q480,540 480,540Q480,540 480,540Z"/>

</vector>
10 changes: 0 additions & 10 deletions app/src/main/res/drawable/ic_overflow_menu.xml

This file was deleted.

5 changes: 5 additions & 0 deletions app/src/main/res/drawable/ic_unarchive.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<vector xmlns:android="http://schemas.android.com/apk/res/android" android:height="24dp" android:tint="?attr/colorControlNormal" android:viewportHeight="24" android:viewportWidth="24" android:width="24dp">

<path android:fillColor="@android:color/white" android:pathData="M20.55,5.22l-1.39,-1.68C18.88,3.21 18.47,3 18,3H6C5.53,3 5.12,3.21 4.85,3.55L3.46,5.22C3.17,5.57 3,6.01 3,6.5V19c0,1.1 0.89,2 2,2h14c1.1,0 2,-0.9 2,-2V6.5C21,6.01 20.83,5.57 20.55,5.22zM12,9.5l5.5,5.5H14v2h-4v-2H6.5L12,9.5zM5.12,5l0.82,-1h12l0.93,1H5.12z"/>

</vector>
38 changes: 12 additions & 26 deletions app/src/main/res/menu/card_view_menu.xml
Original file line number Diff line number Diff line change
Expand Up @@ -15,33 +15,19 @@
app:showAsAction="always" />

<item
android:id="@+id/action_overflow"
android:title="@string/overflowMenu"
android:icon="@drawable/ic_overflow_menu"
app:showAsAction="always">

<menu>

<item
android:id="@+id/action_duplicate"
android:title="@string/duplicateCard"
app:showAsAction="never" />

<item
android:id="@+id/action_archive"
android:title="@string/archive"
app:showAsAction="never"/>
<item
android:id="@+id/action_unarchive"
android:title="@string/unarchive"
app:showAsAction="never"/>
<item
android:id="@+id/action_delete"
android:title="@string/delete"
app:showAsAction="never"/>
android:id="@+id/action_archive_unarchive"
android:title="@string/archive"
android:icon="@drawable/ic_outline_archive"
Comment thread
vijay2909 marked this conversation as resolved.
Outdated
app:showAsAction="always"/>

</menu>
<item
android:id="@+id/action_duplicate"
android:title="@string/duplicateCard"
app:showAsAction="never" />

</item>
<item
android:id="@+id/action_delete"
android:title="@string/delete"
app:showAsAction="never"/>

</menu>
Original file line number Diff line number Diff line change
Expand Up @@ -1034,11 +1034,13 @@ public void checkMenu() throws IOException {
final Menu menu = shadowOf(activity).getOptionsMenu();
assertTrue(menu != null);

// The share, star and overflow options should be present
assertEquals(menu.size(), 3);
assertEquals(menu.size(), 5);

assertEquals("Share", menu.findItem(R.id.action_share).getTitle().toString());
assertEquals("Add to favorites", menu.findItem(R.id.action_star_unstar).getTitle().toString());
assertEquals("Archive", menu.findItem(R.id.action_archive_unarchive).getTitle().toString());
assertEquals("Duplicate", menu.findItem(R.id.action_duplicate).getTitle().toString());
assertEquals("Delete", menu.findItem(R.id.action_delete).getTitle().toString());

database.close();
}
Expand Down Expand Up @@ -1194,8 +1196,7 @@ public void checkPushStarIcon() {
final Menu menu = shadowOf(activity).getOptionsMenu();
assertTrue(menu != null);

// The share, star and overflow options should be present
assertEquals(menu.size(), 3);
assertEquals(menu.size(), 5);

assertEquals("Add to favorites", menu.findItem(R.id.action_star_unstar).getTitle().toString());

Expand All @@ -1207,6 +1208,16 @@ public void checkPushStarIcon() {
shadowOf(getMainLooper()).idle();
assertEquals("Add to favorites", menu.findItem(R.id.action_star_unstar).getTitle().toString());

assertEquals("Archive", menu.findItem(R.id.action_archive_unarchive).getTitle().toString());

shadowOf(activity).clickMenuItem(R.id.action_archive_unarchive);
shadowOf(getMainLooper()).idle();
assertEquals("Unarchive", menu.findItem(R.id.action_archive_unarchive).getTitle().toString());

shadowOf(activity).clickMenuItem(R.id.action_archive_unarchive);
shadowOf(getMainLooper()).idle();
assertEquals("Archive", menu.findItem(R.id.action_archive_unarchive).getTitle().toString());

database.close();
}

Expand Down