Skip to content

Fix position confusion after duplication of rule #70

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
simonpoole merged 1 commit into from
Apr 20, 2026
Merged

Fix position confusion after duplication of rule #70

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 22 additions & 5 deletions lib/src/main/java/ch/poole/openinghoursfragment/OpeningHoursFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1469,7 +1469,7 @@ public boolean onMenuItemClick(MenuItem item) {
MenuItem duplicateRule = menu.add(R.string.duplicate_rule);
duplicateRule.setOnMenuItemClickListener(item -> {
Rule duplicate = r.copy();
int current = rules.indexOf(r);
int current = instancePos(rules, r);
if (current < 0) { // not found shouldn't happen
Log.e(DEBUG_TAG, RULE_MISSING_FROM_LIST);
return true;
Expand All @@ -1480,7 +1480,8 @@ public boolean onMenuItemClick(MenuItem item) {
return true;
});

if (!r.equals(rules.get(0))) { // don't show this for the first rule as it is meaningless
int currentPos = instancePos(rules, r);
if (currentPos != 0) { // don't show this for the first rule as it is meaningless
final View typeView = groupHeader.findViewById(R.id.rule_type_group);
MenuItem showRuleType = menu.add(R.string.rule_type);
showRuleType.setOnMenuItemClickListener(item -> {
Expand All @@ -1490,7 +1491,7 @@ public boolean onMenuItemClick(MenuItem item) {
});
MenuItem moveUp = menu.add(R.string.move_up);
moveUp.setOnMenuItemClickListener(item -> {
int current = rules.indexOf(r);
int current = instancePos(rules, r);
if (current < 0) { // not found shouldn't happen
Log.e(DEBUG_TAG, RULE_MISSING_FROM_LIST);
return true;
Expand All @@ -1502,10 +1503,10 @@ public boolean onMenuItemClick(MenuItem item) {
return true;
});
}
if (!r.equals(rules.get(rules.size() - 1))) {
if (currentPos != (rules.size() - 1)) {
MenuItem moveDown = menu.add(R.string.move_down);
moveDown.setOnMenuItemClickListener(item -> {
int current = rules.indexOf(r);
int current = instancePos(rules, r);
if (current < 0) { // not found shouldn't happen
Log.e(DEBUG_TAG, RULE_MISSING_FROM_LIST);
return true;
Expand Down Expand Up @@ -1608,6 +1609,22 @@ public boolean onMenuItemClick(MenuItem item) {
}
}

/**
* Get the position of o in the list
*
* @param list a List of the objects
* @param o the object we want the position for
* @return the position of o in list or -1
*/
private static <T extends Object> int instancePos(@NonNull final List<T> list, @NonNull final T o) {
for (int i = 0; i < list.size(); i++) {
if (list.get(i) == o) { // NOSONAR, need to find the specific instance so don't use equals
return i;
}
}
return -1;
}

/**
* Helper class to reduce code duplication when adding date range menu items
*
Expand Down
Loading