From 27717750ea22031be83c0d148dc949033189babf Mon Sep 17 00:00:00 2001 From: Lucas Nevado Date: Thu, 3 Sep 2026 16:54:18 -0300 Subject: [PATCH 1/4] Add a per-die lock toggle to the dice screen Each die tile gains a small lock button in its top-end corner. A locked die keeps its value and is skipped by both Roll All and the tap-to-roll gesture on its own tile, so "hold some, reroll the rest" no longer means rerolling each unwanted die by hand. Lock state lives in SavedStateHandle for the session only. Persisting it would be incoherent today, because DicePrefsDataSource stores faces but not values, so a restored "locked" die would show a face-count placeholder rather than the value that was locked. Behaviour details: - Tapping a locked tile shakes it instead of rolling. The card keeps a selectableItemBackground foreground and ripples on touch regardless, so a silent no-op would read as a bug. - Locked tiles do not play the roll animation during Roll All. - Roll All suppresses its haptic and summary bounce when nothing is rollable. This also covers an empty board, which previously buzzed and bounced a total that could not change. - Locked tiles take a 2dp stroke in the luminance-derived contrast colour the adapter already computes, not colorPrimary, which would disappear against neighbouring diceColors entries. - An Unlock All app-bar item appears only while at least one die is locked. There is no Lock All: its only outcome is a dead Roll All. - Lock does not block delete, and the summary chips and total are unchanged. Adds isLocked to the DiffUtil contents check, or lock icons would not refresh when nothing else about a die changed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DvVWNNqvJ3zwwWbA18Uvvq --- .../com/nicue/onetwo/ui/dice/DiceAdapter.java | 151 ++++++++++++++---- .../nicue/onetwo/ui/dice/DiceFragment.java | 42 +++++ .../com/nicue/onetwo/ui/dice/DiceUiState.java | 19 +++ .../nicue/onetwo/ui/dice/DiceViewModel.java | 87 +++++++++- .../com/nicue/onetwo/ui/dice/DieUiModel.java | 8 +- app/src/main/res/drawable/ic_lock_24.xml | 9 ++ app/src/main/res/drawable/ic_lock_open_24.xml | 9 ++ app/src/main/res/layout/dice_item.xml | 12 ++ app/src/main/res/menu/dice_actions.xml | 5 + app/src/main/res/values/strings.xml | 3 + .../onetwo/ui/dice/DiceFragmentTest.java | 57 +++++++ .../onetwo/ui/dice/DiceViewModelTest.java | 129 +++++++++++++++ docs/rfcs/2026-09-03-dice-lock-toggle.md | 99 ++++++++++++ 13 files changed, 593 insertions(+), 37 deletions(-) create mode 100644 app/src/main/res/drawable/ic_lock_24.xml create mode 100644 app/src/main/res/drawable/ic_lock_open_24.xml create mode 100644 docs/rfcs/2026-09-03-dice-lock-toggle.md diff --git a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceAdapter.java b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceAdapter.java index b20b72f..5b42540 100644 --- a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceAdapter.java +++ b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceAdapter.java @@ -1,5 +1,6 @@ package com.nicue.onetwo.ui.dice; +import android.content.res.ColorStateList; import android.view.LayoutInflater; import android.view.View; import android.view.ViewGroup; @@ -17,6 +18,8 @@ public interface Listener { void onRollDie(int position); void onRemoveDie(int position); + + void onToggleLock(int position); } private final Listener listener; @@ -70,7 +73,8 @@ public boolean areContentsTheSame( DieUiModel oldItem = dice.get(oldItemPosition); DieUiModel newItem = newDice.get(newItemPosition); return oldItem.getFaces() == newItem.getFaces() - && oldItem.getValue() == newItem.getValue(); + && oldItem.getValue() == newItem.getValue() + && oldItem.isLocked() == newItem.isLocked(); } }); this.dice = newDice == null ? new ArrayList() : new ArrayList<>(newDice); @@ -82,7 +86,7 @@ public void animateAllVisibleItems(RecyclerView recyclerView, final Runnable end for (int i = 0; i < recyclerView.getChildCount(); i++) { RecyclerView.ViewHolder holder = recyclerView.getChildViewHolder(recyclerView.getChildAt(i)); - if (holder instanceof DiceViewHolder) { + if (isRollable(holder)) { animatedItemCount++; } } @@ -95,7 +99,7 @@ public void animateAllVisibleItems(RecyclerView recyclerView, final Runnable end for (int i = 0; i < recyclerView.getChildCount(); i++) { RecyclerView.ViewHolder holder = recyclerView.getChildViewHolder(recyclerView.getChildAt(i)); - if (holder instanceof DiceViewHolder) { + if (isRollable(holder)) { ((DiceViewHolder) holder) .animateRoll( new Runnable() { @@ -111,8 +115,16 @@ public void run() { } } + private boolean isRollable(RecyclerView.ViewHolder holder) { + return holder instanceof DiceViewHolder && !((DiceViewHolder) holder).isLocked(); + } + class DiceViewHolder extends RecyclerView.ViewHolder { private final DiceItemBinding binding; + private final ColorStateList defaultStrokeColor; + private final int defaultStrokeWidth; + private final int lockedStrokeWidth; + private boolean locked; private final int[] diceColors = { R.color.diceColor0, R.color.diceColor1, R.color.diceColor2, R.color.diceColor3, R.color.diceColor4, R.color.diceColor5, @@ -123,23 +135,32 @@ class DiceViewHolder extends RecyclerView.ViewHolder { DiceViewHolder(DiceItemBinding binding) { super(binding.getRoot()); this.binding = binding; + this.defaultStrokeColor = binding.diceCv.getStrokeColorStateList(); + this.defaultStrokeWidth = binding.diceCv.getStrokeWidth(); + this.lockedStrokeWidth = + Math.round(2 * binding.getRoot().getResources().getDisplayMetrics().density); binding.getRoot() .setOnClickListener( new View.OnClickListener() { @Override public void onClick(View v) { - if (getAdapterPosition() != RecyclerView.NO_POSITION) { - animateRoll( - new Runnable() { - @Override - public void run() { - int position = getAdapterPosition(); - if (position != RecyclerView.NO_POSITION) { - listener.onRollDie(position); - } - } - }); + if (getAdapterPosition() == RecyclerView.NO_POSITION) { + return; + } + if (locked) { + animateLockedNudge(); + return; } + animateRoll( + new Runnable() { + @Override + public void run() { + int position = getAdapterPosition(); + if (position != RecyclerView.NO_POSITION) { + listener.onRollDie(position); + } + } + }); } }); binding.getRoot() @@ -154,14 +175,24 @@ public boolean onLongClick(View v) { return true; } }); + binding.btnLock.setOnClickListener( + new View.OnClickListener() { + @Override + public void onClick(View v) { + int position = getAdapterPosition(); + if (position != RecyclerView.NO_POSITION) { + listener.onToggleLock(position); + } + } + }); + } + + boolean isLocked() { + return locked; } public void animateRoll(final Runnable endAction) { - binding.getRoot().animate().cancel(); - binding.getRoot().setRotation(0f); - binding.getRoot().setScaleX(1f); - binding.getRoot().setScaleY(1f); - binding.getRoot().setTranslationZ(0f); + cancelAndResetTile(); binding.getRoot() .animate() @@ -184,10 +215,7 @@ public void run() { new Runnable() { @Override public void run() { - binding.getRoot().setRotation(0f); - binding.getRoot().setScaleX(1f); - binding.getRoot().setScaleY(1f); - binding.getRoot().setTranslationZ(0f); + resetTileProperties(); if (endAction != null) { endAction.run(); } @@ -199,17 +227,56 @@ public void run() { .start(); } - void bind(DieUiModel dieUiModel) { - int faces = dieUiModel.getFaces(); - int position = getAdapterPosition(); - int colorRes = diceColors[position % diceColors.length]; - int color = binding.getRoot().getContext().getResources().getColor(colorRes); + void animateLockedNudge() { + final View root = binding.getRoot(); + cancelAndResetTile(); + + root.animate() + .translationX(6f) + .setDuration(45) + .withEndAction( + new Runnable() { + @Override + public void run() { + root.animate() + .translationX(-6f) + .setDuration(45) + .withEndAction( + new Runnable() { + @Override + public void run() { + root.animate() + .translationX(0f) + .setDuration(45) + .start(); + } + }) + .start(); + } + }) + .start(); + } + private void cancelAndResetTile() { binding.getRoot().animate().cancel(); + resetTileProperties(); + } + + private void resetTileProperties() { binding.getRoot().setRotation(0f); binding.getRoot().setScaleX(1f); binding.getRoot().setScaleY(1f); + binding.getRoot().setTranslationX(0f); binding.getRoot().setTranslationZ(0f); + } + + void bind(DieUiModel dieUiModel) { + int faces = dieUiModel.getFaces(); + int position = getAdapterPosition(); + int colorRes = diceColors[position % diceColors.length]; + int color = binding.getRoot().getContext().getResources().getColor(colorRes); + + cancelAndResetTile(); binding.diceCv.setCardBackgroundColor(color); @@ -225,6 +292,34 @@ void bind(DieUiModel dieUiModel) { binding.tvDice.setText(String.valueOf(dieUiModel.getValue())); binding.tvDieType.setText( binding.getRoot().getContext().getString(R.string.dice_type_label, faces)); + + bindLockState(dieUiModel.isLocked(), textColor); + } + + private void bindLockState(boolean isLocked, int contrastColor) { + this.locked = isLocked; + + binding.btnLock.setImageResource( + isLocked ? R.drawable.ic_lock_24 : R.drawable.ic_lock_open_24); + binding.btnLock.setColorFilter(contrastColor); + binding.btnLock.setAlpha(isLocked ? 1f : 0.5f); + binding.btnLock.setContentDescription( + binding.getRoot() + .getContext() + .getString( + isLocked + ? R.string.content_desc_unlock_die + : R.string.content_desc_lock_die)); + + if (isLocked) { + binding.diceCv.setStrokeColor(contrastColor); + binding.diceCv.setStrokeWidth(lockedStrokeWidth); + } else { + if (defaultStrokeColor != null) { + binding.diceCv.setStrokeColor(defaultStrokeColor); + } + binding.diceCv.setStrokeWidth(defaultStrokeWidth); + } } @DrawableRes diff --git a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceFragment.java b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceFragment.java index 7a0fd31..bf94a02 100644 --- a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceFragment.java +++ b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceFragment.java @@ -34,6 +34,8 @@ public class DiceFragment extends Fragment implements DiceAdapter.Listener, Menu private DiceLayoutBinding binding; private DiceAdapter adapter; private DiceViewModel viewModel; + private boolean hasLockedDice; + private boolean hasRollableDice; @Nullable @Override public View onCreateView( @@ -97,6 +99,7 @@ public void onClick(View v) { public void onChanged(DiceUiState state) { adapter.submitList(state.getDice()); renderResultSummary(state); + updateLockActionState(state); } }); @@ -120,9 +123,24 @@ public void onCreateMenu(@NonNull Menu menu, @NonNull MenuInflater menuInflater) menuInflater.inflate(R.menu.dice_actions, menu); } + @Override + public void onPrepareMenu(@NonNull Menu menu) { + MenuItem unlockAll = menu.findItem(R.id.action_unlock_all); + if (unlockAll != null) { + unlockAll.setVisible(hasLockedDice); + } + } + @Override public boolean onMenuItemSelected(@NonNull MenuItem menuItem) { + if (menuItem.getItemId() == R.id.action_unlock_all) { + viewModel.unlockAllDice(); + return true; + } if (menuItem.getItemId() == R.id.action_roll_all) { + if (!hasRollableDice) { + return true; + } vibrate(new long[] {0, 15, 10, 15, 10, 15, 10, 15}); animateSummaryCard(); adapter.animateAllVisibleItems( @@ -180,6 +198,23 @@ public void onRemoveDie(int position) { viewModel.removeDie(position); } + @Override + public void onToggleLock(int position) { + if (binding == null) { + return; + } + vibrate(30L); + viewModel.toggleLock(position); + } + + private void updateLockActionState(DiceUiState state) { + hasRollableDice = state.hasRollableDice(); + if (hasLockedDice != state.hasLockedDice()) { + hasLockedDice = state.hasLockedDice(); + requireActivity().invalidateMenu(); + } + } + public static String normalizeFacesInput(String input) { if (input == null || input.trim().isEmpty()) { return "6"; @@ -277,6 +312,13 @@ private void renderResultSummary(DiceUiState state) { } } + private void vibrate(long milliseconds) { + Vibrator vibrator = (Vibrator) requireContext().getSystemService(Context.VIBRATOR_SERVICE); + if (vibrator != null) { + vibrator.vibrate(milliseconds); + } + } + private void vibrate(long[] pattern) { Vibrator vibrator = (Vibrator) requireContext().getSystemService(Context.VIBRATOR_SERVICE); if (vibrator != null) { diff --git a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceUiState.java b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceUiState.java index af36299..ae874f8 100644 --- a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceUiState.java +++ b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceUiState.java @@ -6,14 +6,25 @@ public class DiceUiState { private final ArrayList dice; private final int total; + private final boolean hasLockedDice; + private final boolean hasRollableDice; public DiceUiState(List dice) { this.dice = new ArrayList<>(dice); int total = 0; + boolean hasLockedDice = false; + boolean hasRollableDice = false; for (DieUiModel die : dice) { total += die.getValue(); + if (die.isLocked()) { + hasLockedDice = true; + } else { + hasRollableDice = true; + } } this.total = total; + this.hasLockedDice = hasLockedDice; + this.hasRollableDice = hasRollableDice; } public ArrayList getDice() { @@ -23,4 +34,12 @@ public ArrayList getDice() { public int getTotal() { return total; } + + public boolean hasLockedDice() { + return hasLockedDice; + } + + public boolean hasRollableDice() { + return hasRollableDice; + } } diff --git a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceViewModel.java b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceViewModel.java index a0dd9ad..d75eeff 100644 --- a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceViewModel.java +++ b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceViewModel.java @@ -13,6 +13,7 @@ public class DiceViewModel extends ViewModel { private static final String KEY_FACES = "dice_faces"; private static final String KEY_VALUES = "dice_values"; private static final String KEY_IDS = "dice_ids"; + private static final String KEY_LOCKED = "dice_locked"; private static final String KEY_NEXT_ID = "dice_next_id"; private final DiceRepository diceRepository; @@ -27,7 +28,7 @@ public DiceViewModel(DiceRepository diceRepository, SavedStateHandle savedStateH ArrayList savedFaces = savedStateHandle.get(KEY_FACES); ArrayList savedValues = savedStateHandle.get(KEY_VALUES); ArrayList savedIds = savedStateHandle.get(KEY_IDS); - Long nextId = savedStateHandle.get(KEY_NEXT_ID); + ArrayList savedLocks = savedStateHandle.get(KEY_LOCKED); if (savedFaces == null) { savedFaces = new ArrayList<>(diceRepository.readDiceFaces()); @@ -46,7 +47,13 @@ public DiceViewModel(DiceRepository diceRepository, SavedStateHandle savedStateH } savedStateHandle.set(KEY_NEXT_ID, currentId); } - updateState(savedFaces, savedValues, savedIds); + if (savedLocks == null || savedLocks.size() != savedFaces.size()) { + savedLocks = new ArrayList<>(); + for (int i = 0; i < savedFaces.size(); i++) { + savedLocks.add(Boolean.FALSE); + } + } + updateState(savedFaces, savedValues, savedIds, savedLocks); } public LiveData getUiState() { @@ -57,6 +64,7 @@ public void addDie(int faces) { ArrayList dieFaces = getFaces(); ArrayList dieValues = getValues(); ArrayList dieIds = getIds(); + ArrayList dieLocks = getLocks(); Long nextId = savedStateHandle.get(KEY_NEXT_ID); if (nextId == null) { nextId = 0L; @@ -65,45 +73,94 @@ public void addDie(int faces) { dieFaces.add(Math.max(2, faces)); dieValues.add(Math.max(2, faces)); dieIds.add(nextId); + dieLocks.add(Boolean.FALSE); savedStateHandle.set(KEY_NEXT_ID, nextId + 1); persistFaces(dieFaces); - updateState(dieFaces, dieValues, dieIds); + updateState(dieFaces, dieValues, dieIds, dieLocks); } public void removeDie(int position) { ArrayList dieFaces = getFaces(); ArrayList dieValues = getValues(); ArrayList dieIds = getIds(); + ArrayList dieLocks = getLocks(); if (position < 0 || position >= dieFaces.size()) { return; } dieFaces.remove(position); dieValues.remove(position); dieIds.remove(position); + dieLocks.remove(position); persistFaces(dieFaces); - updateState(dieFaces, dieValues, dieIds); + updateState(dieFaces, dieValues, dieIds, dieLocks); } public void rollDie(int position) { ArrayList dieFaces = getFaces(); ArrayList dieValues = getValues(); ArrayList dieIds = getIds(); + ArrayList dieLocks = getLocks(); if (position < 0 || position >= dieFaces.size()) { return; } + if (isLocked(dieLocks, position)) { + return; + } dieValues.set(position, roll(dieFaces.get(position))); - updateState(dieFaces, dieValues, dieIds); + updateState(dieFaces, dieValues, dieIds, dieLocks); } public void rollAllDice() { ArrayList dieFaces = getFaces(); ArrayList dieValues = getValues(); ArrayList dieIds = getIds(); + ArrayList dieLocks = getLocks(); for (int i = 0; i < dieFaces.size(); i++) { + if (isLocked(dieLocks, i)) { + continue; + } dieValues.set(i, roll(dieFaces.get(i))); } - updateState(dieFaces, dieValues, dieIds); + updateState(dieFaces, dieValues, dieIds, dieLocks); + } + + public void toggleLock(int position) { + ArrayList dieFaces = getFaces(); + ArrayList dieValues = getValues(); + ArrayList dieIds = getIds(); + ArrayList dieLocks = getLocks(); + if (position < 0 || position >= dieLocks.size()) { + return; + } + dieLocks.set(position, !isLocked(dieLocks, position)); + updateState(dieFaces, dieValues, dieIds, dieLocks); + } + + public void unlockAllDice() { + ArrayList dieFaces = getFaces(); + ArrayList dieValues = getValues(); + ArrayList dieIds = getIds(); + ArrayList dieLocks = getLocks(); + boolean changed = false; + for (int i = 0; i < dieLocks.size(); i++) { + if (isLocked(dieLocks, i)) { + dieLocks.set(i, Boolean.FALSE); + changed = true; + } + } + if (!changed) { + return; + } + updateState(dieFaces, dieValues, dieIds, dieLocks); + } + + private boolean isLocked(List dieLocks, int position) { + if (position < 0 || position >= dieLocks.size()) { + return false; + } + Boolean locked = dieLocks.get(position); + return locked != null && locked; } private int roll(int faces) { @@ -129,14 +186,28 @@ private ArrayList getIds() { return values == null ? new ArrayList() : new ArrayList<>(values); } + private ArrayList getLocks() { + ArrayList values = savedStateHandle.get(KEY_LOCKED); + return values == null ? new ArrayList() : new ArrayList<>(values); + } + private void updateState( - ArrayList dieFaces, ArrayList dieValues, ArrayList dieIds) { + ArrayList dieFaces, + ArrayList dieValues, + ArrayList dieIds, + ArrayList dieLocks) { savedStateHandle.set(KEY_FACES, new ArrayList<>(dieFaces)); savedStateHandle.set(KEY_VALUES, new ArrayList<>(dieValues)); savedStateHandle.set(KEY_IDS, new ArrayList<>(dieIds)); + savedStateHandle.set(KEY_LOCKED, new ArrayList<>(dieLocks)); ArrayList dice = new ArrayList<>(); for (int i = 0; i < dieFaces.size(); i++) { - dice.add(new DieUiModel(dieIds.get(i), dieFaces.get(i), dieValues.get(i))); + dice.add( + new DieUiModel( + dieIds.get(i), + dieFaces.get(i), + dieValues.get(i), + isLocked(dieLocks, i))); } uiState.setValue(new DiceUiState(dice)); } diff --git a/app/src/main/java/com/nicue/onetwo/ui/dice/DieUiModel.java b/app/src/main/java/com/nicue/onetwo/ui/dice/DieUiModel.java index ecc9fba..9093f45 100644 --- a/app/src/main/java/com/nicue/onetwo/ui/dice/DieUiModel.java +++ b/app/src/main/java/com/nicue/onetwo/ui/dice/DieUiModel.java @@ -4,11 +4,13 @@ public class DieUiModel { private final long id; private final int faces; private final int value; + private final boolean locked; - public DieUiModel(long id, int faces, int value) { + public DieUiModel(long id, int faces, int value, boolean locked) { this.id = id; this.faces = faces; this.value = value; + this.locked = locked; } public long getId() { @@ -22,4 +24,8 @@ public int getFaces() { public int getValue() { return value; } + + public boolean isLocked() { + return locked; + } } diff --git a/app/src/main/res/drawable/ic_lock_24.xml b/app/src/main/res/drawable/ic_lock_24.xml new file mode 100644 index 0000000..966e9a4 --- /dev/null +++ b/app/src/main/res/drawable/ic_lock_24.xml @@ -0,0 +1,9 @@ + + + diff --git a/app/src/main/res/drawable/ic_lock_open_24.xml b/app/src/main/res/drawable/ic_lock_open_24.xml new file mode 100644 index 0000000..801ee72 --- /dev/null +++ b/app/src/main/res/drawable/ic_lock_open_24.xml @@ -0,0 +1,9 @@ + + + diff --git a/app/src/main/res/layout/dice_item.xml b/app/src/main/res/layout/dice_item.xml index be16bcf..4c8506f 100644 --- a/app/src/main/res/layout/dice_item.xml +++ b/app/src/main/res/layout/dice_item.xml @@ -46,6 +46,18 @@ app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toTopOf="parent" /> + + + Edit Roll Roll All + Unlock All Total Add dice to start rolling d%1$d @@ -90,6 +91,8 @@ : Add Counter Roll Dice + Lock die + Unlock die Life Counter New Game diff --git a/app/src/test/java/com/nicue/onetwo/ui/dice/DiceFragmentTest.java b/app/src/test/java/com/nicue/onetwo/ui/dice/DiceFragmentTest.java index 31bdd36..cc99222 100644 --- a/app/src/test/java/com/nicue/onetwo/ui/dice/DiceFragmentTest.java +++ b/app/src/test/java/com/nicue/onetwo/ui/dice/DiceFragmentTest.java @@ -1,8 +1,12 @@ package com.nicue.onetwo.ui.dice; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; import android.view.View; +import android.widget.ImageButton; import android.widget.TextView; import androidx.arch.core.executor.testing.InstantTaskExecutorRule; import androidx.fragment.app.testing.FragmentScenario; @@ -74,4 +78,57 @@ public void addDieViaViewModel_updatesUI() { assertEquals(View.GONE, emptyText.getVisibility()); }); } + + @Test + public void lockButton_togglesLockStateAndUpdatesTile() { + FragmentScenario scenario = + FragmentScenario.launchInContainer(DiceFragment.class, null, R.style.AppTheme); + + scenario.onFragment( + fragment -> { + DiceViewModel viewModel = + new androidx.lifecycle.ViewModelProvider(fragment) + .get(DiceViewModel.class); + while (!viewModel.getUiState().getValue().getDice().isEmpty()) { + viewModel.removeDie(0); + } + viewModel.addDie(6); + + RecyclerView recyclerView = + fragment.getView().findViewById(R.id.recyclerview_dice); + layOut(recyclerView); + + View tile = recyclerView.getChildAt(0); + assertNotNull(tile); + ImageButton lockButton = tile.findViewById(R.id.btn_lock); + assertEquals( + fragment.getString(R.string.content_desc_lock_die), + lockButton.getContentDescription().toString()); + + lockButton.performClick(); + + DiceUiState locked = viewModel.getUiState().getValue(); + assertTrue(locked.getDice().get(0).isLocked()); + assertTrue(locked.hasLockedDice()); + assertFalse(locked.hasRollableDice()); + + layOut(recyclerView); + ImageButton lockedButton = + recyclerView.getChildAt(0).findViewById(R.id.btn_lock); + assertEquals( + fragment.getString(R.string.content_desc_unlock_die), + lockedButton.getContentDescription().toString()); + + lockedButton.performClick(); + + assertFalse(viewModel.getUiState().getValue().getDice().get(0).isLocked()); + }); + } + + private static void layOut(View view) { + view.measure( + View.MeasureSpec.makeMeasureSpec(1080, View.MeasureSpec.EXACTLY), + View.MeasureSpec.makeMeasureSpec(1920, View.MeasureSpec.EXACTLY)); + view.layout(0, 0, 1080, 1920); + } } diff --git a/app/src/test/java/com/nicue/onetwo/ui/dice/DiceViewModelTest.java b/app/src/test/java/com/nicue/onetwo/ui/dice/DiceViewModelTest.java index de4ac93..3966202 100644 --- a/app/src/test/java/com/nicue/onetwo/ui/dice/DiceViewModelTest.java +++ b/app/src/test/java/com/nicue/onetwo/ui/dice/DiceViewModelTest.java @@ -1,6 +1,7 @@ package com.nicue.onetwo.ui.dice; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import android.content.Context; @@ -63,4 +64,132 @@ public void addRemoveRollAll_updatesDiceState() throws Exception { assertEquals(20, dice.get(0).getFaces()); assertEquals(dice.get(0).getValue(), state.getTotal()); } + + @Test + public void rollAllDice_keepsLockedDieValue() throws Exception { + viewModel.addDie(20); + viewModel.addDie(20); + viewModel.toggleLock(0); + + int lockedValue = dieAt(0).getValue(); + for (int i = 0; i < 50; i++) { + viewModel.rollAllDice(); + assertEquals(lockedValue, dieAt(0).getValue()); + } + assertTrue(dieAt(0).isLocked()); + assertFalse(dieAt(1).isLocked()); + } + + @Test + public void rollDie_ignoresLockedDie() throws Exception { + viewModel.addDie(20); + viewModel.toggleLock(0); + + int lockedValue = dieAt(0).getValue(); + for (int i = 0; i < 50; i++) { + viewModel.rollDie(0); + assertEquals(lockedValue, dieAt(0).getValue()); + } + } + + @Test + public void toggleLock_unlocksAndAllowsRollingAgain() throws Exception { + viewModel.addDie(6); + viewModel.toggleLock(0); + assertTrue(dieAt(0).isLocked()); + + viewModel.toggleLock(0); + assertFalse(dieAt(0).isLocked()); + } + + @Test + public void addDie_startsUnlocked() throws Exception { + viewModel.addDie(6); + viewModel.toggleLock(0); + viewModel.addDie(6); + + assertTrue(dieAt(0).isLocked()); + assertFalse(dieAt(1).isLocked()); + } + + @Test + public void removeDie_shiftsLockFlagsWithRemainingDice() throws Exception { + viewModel.addDie(4); + viewModel.addDie(6); + viewModel.addDie(20); + viewModel.toggleLock(2); + + viewModel.removeDie(0); + + DiceUiState state = LiveDataTestUtil.getValue(viewModel.getUiState()); + assertEquals(2, state.getDice().size()); + assertEquals(6, dieAt(0).getFaces()); + assertFalse(dieAt(0).isLocked()); + assertEquals(20, dieAt(1).getFaces()); + assertTrue(dieAt(1).isLocked()); + } + + @Test + public void lockState_survivesSavedStateHandleRoundTrip() throws Exception { + SavedStateHandle savedStateHandle = new SavedStateHandle(); + DiceRepository repository = + new DiceRepository( + new DicePrefsDataSource( + ApplicationProvider.getApplicationContext())); + DiceViewModel first = new DiceViewModel(repository, savedStateHandle); + first.addDie(6); + first.addDie(20); + first.toggleLock(1); + + DiceViewModel restored = new DiceViewModel(repository, savedStateHandle); + + List dice = LiveDataTestUtil.getValue(restored.getUiState()).getDice(); + assertEquals(2, dice.size()); + assertFalse(dice.get(0).isLocked()); + assertTrue(dice.get(1).isLocked()); + } + + @Test + public void unlockAllDice_clearsEveryLock() throws Exception { + viewModel.addDie(6); + viewModel.addDie(20); + viewModel.toggleLock(0); + viewModel.toggleLock(1); + + viewModel.unlockAllDice(); + + assertFalse(dieAt(0).isLocked()); + assertFalse(dieAt(1).isLocked()); + } + + @Test + public void uiState_reportsLockedAndRollableDice() throws Exception { + DiceUiState state = LiveDataTestUtil.getValue(viewModel.getUiState()); + assertFalse(state.hasLockedDice()); + assertFalse(state.hasRollableDice()); + + viewModel.addDie(6); + state = LiveDataTestUtil.getValue(viewModel.getUiState()); + assertFalse(state.hasLockedDice()); + assertTrue(state.hasRollableDice()); + + viewModel.toggleLock(0); + state = LiveDataTestUtil.getValue(viewModel.getUiState()); + assertTrue(state.hasLockedDice()); + assertFalse(state.hasRollableDice()); + } + + @Test + public void lockedDie_stillCountsTowardTotal() throws Exception { + viewModel.addDie(6); + viewModel.addDie(6); + viewModel.toggleLock(0); + + DiceUiState state = LiveDataTestUtil.getValue(viewModel.getUiState()); + assertEquals(dieAt(0).getValue() + dieAt(1).getValue(), state.getTotal()); + } + + private DieUiModel dieAt(int position) throws Exception { + return LiveDataTestUtil.getValue(viewModel.getUiState()).getDice().get(position); + } } diff --git a/docs/rfcs/2026-09-03-dice-lock-toggle.md b/docs/rfcs/2026-09-03-dice-lock-toggle.md new file mode 100644 index 0000000..2865c24 --- /dev/null +++ b/docs/rfcs/2026-09-03-dice-lock-toggle.md @@ -0,0 +1,99 @@ +# RFC: Dice Lock Toggle + +- Status: Proposed +- Date: 2026-09-03 + +## Summary + +Add a per-die lock toggle to the dice screen. Each die tile gains a small lock button in its top-end corner. A locked die is frozen: it keeps its current value and is skipped by both `Roll All` and the tap-to-roll gesture on its own tile. Lock state lives in `SavedStateHandle` for the current session only and is not persisted to `SharedPreferences`. + +## Motivation + +The dice screen currently rolls all-or-one. Many tabletop games need the "hold some, reroll the rest" flow: Yahtzee-style keeps, re-rolling a single bad d20 while the damage dice stand, or holding a result while deciding what to do next. + +Today the only way to preserve a value is to not press `Roll All`, then reroll each unwanted die individually by tapping it. That is tedious and error prone, because a stray tap on a tile silently rerolls it with no way to recover the previous value. + +## Goals + +- Let the user mark any die as locked and unlocked from the tile itself. +- Exclude locked dice from `Roll All`. +- Exclude locked dice from the single-die tap-to-roll gesture. +- Make lock state legible at a glance across a grid of tiles. +- Preserve lock state across configuration changes. +- Keep the existing roll, add, and remove interactions unchanged. + +## Non-Goals + +- Persisting lock state, or die values, across process death. +- Protecting locked dice from deletion. +- A `lock all` action. +- Locked/unlocked breakdowns in the results summary or total. +- Any change to the persisted dice format in `DicePrefsDataSource`. + +## Product Decisions + +### Lock Semantics + +A lock blocks every reroll of that die, not just `Roll All`. + +The tile root is one large roll target (`DiceAdapter` binds a click listener to the card root), so a lock that only guarded the toolbar action would be defeated by any stray tap on the tile. A lock the user can lose by accident is not worth having. + +Tapping a locked tile is not silent. The card keeps `?attr/selectableItemBackground` as its foreground, so it ripples on touch whether or not anything happens; a ripple with no consequence reads as a bug. A locked tile answers a tap with a short shake instead of the roll spin. + +### Lock Does Not Block Delete + +Long-press still removes a locked die immediately. + +Lock means "this value is held", not "this tile is protected from everything". Overloading it into general write protection makes the icon mean two things at once, and long-press is deliberate enough that accidental deletion of a locked die is unlikely. + +Note that long-press delete has no confirmation and no undo today. That is a pre-existing rough edge and is out of scope for this RFC. + +### State Lifetime + +Lock state is held in `SavedStateHandle` alongside faces, values, and ids. It survives rotation and view recreation. It does not survive process death. + +Locking is a within-a-session action: hold these, reroll those. Persisting it to `SharedPreferences` would be actively wrong today, because only faces are persisted and values are not — a die restored as "locked" would display a face-count placeholder rather than the value the user locked, which is incoherent. Persisting values as well, purely to support locks across app restarts, is a larger change serving a case nobody has asked for. + +### Roll All With Nothing Rollable + +If every die is locked, `Roll All` suppresses its haptic burst and the summary-card bounce entirely. + +The default path fires an eight-pulse vibration and animates the total before rolling. Playing that when no value can change tells the user something happened when nothing did. + +The same suppression applies when the board is empty, which is a small behaviour change beyond locking: `Roll All` on a board with no dice previously buzzed and bounced the total too. Both cases are the same statement — nothing on this board can change — and should read the same way. + +### Tile Treatment + +The lock control is a small always-present `ImageButton` pinned to the tile's top-end corner, consuming its own clicks so it never triggers a roll. Top-start houses the die-type badge, the centre holds the value, and bottom-end holds the die glyph, so top-end is the one free corner. + +The icon is recessive when unlocked (alpha ~0.5, matching the die glyph's existing treatment on the same tile) and full opacity when locked. It stays visible in both states: a control that only appears once used is undiscoverable. + +Icon alone is not enough. On a two-column grid with up to ten tile colours, a 20dp corner glyph is easy to miss when scanning which dice are held, so a locked tile also takes a 2dp stroke. + +That stroke uses the luminance-derived contrast colour `DiceAdapter` already computes for tile text, not `?attr/colorPrimary`. Tile backgrounds are assigned at runtime from `diceColors`, and a fixed theme colour will disappear against whichever entries sit near it. The same computed colour tints the lock icon. + +### Unlock All + +A second app-bar item appears beside `Roll All`, but only when at least one die is locked. + +With several dice locked, clearing them means several precise taps on small corner targets. There is no matching `lock all`: its only outcome is a state where `Roll All` does nothing, which is a control whose success case is a dead button. + +### Summary And Total + +The results summary and total are unchanged. A locked die contributes its held value to the total and renders an ordinary chip. + +The total answers "what is on the table", and a locked die is still on the table. The chips are already tight at `minWidth=56dp` and a 28dp height, with no room for a state glyph. + +## Implementation Notes + +- `DiceViewModel` gains a fourth parallel array, `locked`, kept in sync with faces, values, and ids across `addDie`, `removeDie`, `rollDie`, and `rollAllDice`. Removal at a lower index must shift lock flags with the rest of the state. +- `DieUiModel` gains `isLocked()`. `DiceAdapter`'s `DiffUtil.areContentsTheSame` must compare it, or lock icons will not refresh when nothing else about the die changed. +- `DiceAdapter.Listener` gains `onToggleLock(int position)`. +- `animateAllVisibleItems` skips locked holders. Its completion counter must still fire the end action when zero tiles animate, otherwise an all-locked `Roll All` would hang the callback. +- Two new vector drawables, `ic_lock_24` and `ic_lock_open_24`. +- Toggling a lock fires a single short vibration, matching the tap convention used elsewhere in the app. `DiceFragment` currently exposes only a `long[]` pattern overload and needs the scalar form. + +## Testing + +- `DiceViewModelTest`: locked dice keep their value through `rollAllDice` and `rollDie`; lock state survives a `SavedStateHandle` round trip; lock flags shift correctly when a die at a lower index is removed. +- `DiceFragmentTest`: the lock button toggles the die's state and the tile reflects it. From c05cbb0cb01a4e536fbbc714b41568f48537ff1d Mon Sep 17 00:00:00 2001 From: Lucas Nevado Date: Thu, 3 Sep 2026 17:04:07 -0300 Subject: [PATCH 2/4] Fix dice lock feedback gaps found in review Three defects in the lock toggle, all cases where a control did something invisible or nothing at all. The locked-tile shake used raw pixels. setTranslationX takes pixels, not dp, so the 6f nudge was about 2dp on a 3x-density screen. Since the locked path also returns before any haptic fires, tapping a locked tile produced a ripple and no other feedback, which is exactly the "reads as a bug" outcome the shake existed to prevent. Now scaled by display density, matching how the same class already computes the locked stroke width. The lock button swallowed long presses. A clickable child consumes the touch stream, and the button is not long-clickable, so a long press in the top-end corner never reached the card's delete handler: the gesture deleted the die everywhere on the tile except a 44dp square. The button now forwards long presses to the same handler. Roll All stayed enabled while inert. When nothing is rollable the action suppressed its haptic, animation and roll, but still looked available. onPrepareMenu now disables it, so the dead state is visible rather than silent. This also required invalidating the menu when the rollable flag changes on its own, not just the locked flag; otherwise Roll All stayed disabled after adding the first die to an empty board. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DvVWNNqvJ3zwwWbA18Uvvq --- .../com/nicue/onetwo/ui/dice/DiceAdapter.java | 23 +++++-- .../nicue/onetwo/ui/dice/DiceFragment.java | 10 ++- .../onetwo/ui/dice/DiceFragmentTest.java | 69 +++++++++++++++++++ 3 files changed, 96 insertions(+), 6 deletions(-) diff --git a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceAdapter.java b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceAdapter.java index 5b42540..bacc020 100644 --- a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceAdapter.java +++ b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceAdapter.java @@ -124,6 +124,7 @@ class DiceViewHolder extends RecyclerView.ViewHolder { private final ColorStateList defaultStrokeColor; private final int defaultStrokeWidth; private final int lockedStrokeWidth; + private final float nudgeTranslation; private boolean locked; private final int[] diceColors = { R.color.diceColor0, R.color.diceColor1, R.color.diceColor2, @@ -137,8 +138,9 @@ class DiceViewHolder extends RecyclerView.ViewHolder { this.binding = binding; this.defaultStrokeColor = binding.diceCv.getStrokeColorStateList(); this.defaultStrokeWidth = binding.diceCv.getStrokeWidth(); - this.lockedStrokeWidth = - Math.round(2 * binding.getRoot().getResources().getDisplayMetrics().density); + float density = binding.getRoot().getResources().getDisplayMetrics().density; + this.lockedStrokeWidth = Math.round(2 * density); + this.nudgeTranslation = 6 * density; binding.getRoot() .setOnClickListener( new View.OnClickListener() { @@ -185,6 +187,19 @@ public void onClick(View v) { } } }); + // The button consumes touches, so without this a long press on the + // lock corner would be swallowed instead of removing the die. + binding.btnLock.setOnLongClickListener( + new View.OnLongClickListener() { + @Override + public boolean onLongClick(View v) { + int position = getAdapterPosition(); + if (position != RecyclerView.NO_POSITION) { + listener.onRemoveDie(position); + } + return true; + } + }); } boolean isLocked() { @@ -232,14 +247,14 @@ void animateLockedNudge() { cancelAndResetTile(); root.animate() - .translationX(6f) + .translationX(nudgeTranslation) .setDuration(45) .withEndAction( new Runnable() { @Override public void run() { root.animate() - .translationX(-6f) + .translationX(-nudgeTranslation) .setDuration(45) .withEndAction( new Runnable() { diff --git a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceFragment.java b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceFragment.java index bf94a02..d71b50b 100644 --- a/app/src/main/java/com/nicue/onetwo/ui/dice/DiceFragment.java +++ b/app/src/main/java/com/nicue/onetwo/ui/dice/DiceFragment.java @@ -129,6 +129,10 @@ public void onPrepareMenu(@NonNull Menu menu) { if (unlockAll != null) { unlockAll.setVisible(hasLockedDice); } + MenuItem rollAll = menu.findItem(R.id.action_roll_all); + if (rollAll != null) { + rollAll.setEnabled(hasRollableDice); + } } @Override @@ -208,9 +212,11 @@ public void onToggleLock(int position) { } private void updateLockActionState(DiceUiState state) { + boolean lockedChanged = hasLockedDice != state.hasLockedDice(); + boolean rollableChanged = hasRollableDice != state.hasRollableDice(); + hasLockedDice = state.hasLockedDice(); hasRollableDice = state.hasRollableDice(); - if (hasLockedDice != state.hasLockedDice()) { - hasLockedDice = state.hasLockedDice(); + if (lockedChanged || rollableChanged) { requireActivity().invalidateMenu(); } } diff --git a/app/src/test/java/com/nicue/onetwo/ui/dice/DiceFragmentTest.java b/app/src/test/java/com/nicue/onetwo/ui/dice/DiceFragmentTest.java index cc99222..db0c197 100644 --- a/app/src/test/java/com/nicue/onetwo/ui/dice/DiceFragmentTest.java +++ b/app/src/test/java/com/nicue/onetwo/ui/dice/DiceFragmentTest.java @@ -5,8 +5,10 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; +import android.view.Menu; import android.view.View; import android.widget.ImageButton; +import android.widget.PopupMenu; import android.widget.TextView; import androidx.arch.core.executor.testing.InstantTaskExecutorRule; import androidx.fragment.app.testing.FragmentScenario; @@ -131,4 +133,71 @@ private static void layOut(View view) { View.MeasureSpec.makeMeasureSpec(1920, View.MeasureSpec.EXACTLY)); view.layout(0, 0, 1080, 1920); } + + @Test + public void lockButtonLongPress_stillRemovesTheDie() { + FragmentScenario scenario = + FragmentScenario.launchInContainer(DiceFragment.class, null, R.style.AppTheme); + + scenario.onFragment( + fragment -> { + DiceViewModel viewModel = clearedViewModel(fragment); + viewModel.addDie(6); + + RecyclerView recyclerView = + fragment.getView().findViewById(R.id.recyclerview_dice); + layOut(recyclerView); + + ImageButton lockButton = recyclerView.getChildAt(0).findViewById(R.id.btn_lock); + lockButton.performLongClick(); + + assertTrue(viewModel.getUiState().getValue().getDice().isEmpty()); + }); + } + + @Test + public void rollAllAction_isDisabledWhenNothingCanBeRolled() { + FragmentScenario scenario = + FragmentScenario.launchInContainer(DiceFragment.class, null, R.style.AppTheme); + + scenario.onFragment( + fragment -> { + DiceViewModel viewModel = clearedViewModel(fragment); + + Menu menu = inflateDiceMenu(fragment); + fragment.onPrepareMenu(menu); + assertFalse(menu.findItem(R.id.action_roll_all).isEnabled()); + assertFalse(menu.findItem(R.id.action_unlock_all).isVisible()); + + viewModel.addDie(6); + fragment.onPrepareMenu(menu); + assertTrue(menu.findItem(R.id.action_roll_all).isEnabled()); + assertFalse(menu.findItem(R.id.action_unlock_all).isVisible()); + + viewModel.toggleLock(0); + fragment.onPrepareMenu(menu); + assertFalse(menu.findItem(R.id.action_roll_all).isEnabled()); + assertTrue(menu.findItem(R.id.action_unlock_all).isVisible()); + + viewModel.unlockAllDice(); + fragment.onPrepareMenu(menu); + assertTrue(menu.findItem(R.id.action_roll_all).isEnabled()); + assertFalse(menu.findItem(R.id.action_unlock_all).isVisible()); + }); + } + + private static Menu inflateDiceMenu(DiceFragment fragment) { + PopupMenu popupMenu = new PopupMenu(fragment.requireContext(), fragment.getView()); + popupMenu.getMenuInflater().inflate(R.menu.dice_actions, popupMenu.getMenu()); + return popupMenu.getMenu(); + } + + private static DiceViewModel clearedViewModel(DiceFragment fragment) { + DiceViewModel viewModel = + new androidx.lifecycle.ViewModelProvider(fragment).get(DiceViewModel.class); + while (!viewModel.getUiState().getValue().getDice().isEmpty()) { + viewModel.removeDie(0); + } + return viewModel; + } } From 9bd5bcec60ca7e720befe7fcc219135190ac25e1 Mon Sep 17 00:00:00 2001 From: Lucas Nevado Date: Thu, 3 Sep 2026 17:47:01 -0300 Subject: [PATCH 3/4] Add debug build configuration and strings resource --- app/build.gradle | 6 ++++++ app/src/debug/res/values/strings.xml | 4 ++++ 2 files changed, 10 insertions(+) create mode 100644 app/src/debug/res/values/strings.xml diff --git a/app/build.gradle b/app/build.gradle index 1dd0acf..5e28df9 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -19,6 +19,12 @@ android { testInstrumentationRunner 'androidx.test.runner.AndroidJUnitRunner' } buildTypes { + debug { + // Distinct applicationId so debug builds install alongside a + // release install of OneTwo rather than replacing it. + applicationIdSuffix ".debug" + versionNameSuffix "-debug" + } release { minifyEnabled true proguardFiles getDefaultProguardFile('proguard-android-optimize.txt'), 'proguard-rules.pro' diff --git a/app/src/debug/res/values/strings.xml b/app/src/debug/res/values/strings.xml new file mode 100644 index 0000000..8e18e1d --- /dev/null +++ b/app/src/debug/res/values/strings.xml @@ -0,0 +1,4 @@ + + + OneTwo Debug + From e6e5b400bb9dfb7ad2470dd63c963c0d544afefc Mon Sep 17 00:00:00 2001 From: Lucas Nevado Date: Thu, 3 Sep 2026 17:55:36 -0300 Subject: [PATCH 4/4] Add unlock all string resource for Spanish, French, and Japanese translations --- app/src/main/res/values-es/strings.xml | 1 + app/src/main/res/values-fr/strings.xml | 1 + app/src/main/res/values-ja/strings.xml | 1 + 3 files changed, 3 insertions(+) diff --git a/app/src/main/res/values-es/strings.xml b/app/src/main/res/values-es/strings.xml index 9cdc595..788a621 100644 --- a/app/src/main/res/values-es/strings.xml +++ b/app/src/main/res/values-es/strings.xml @@ -32,6 +32,7 @@ Editar Tirar Tirar todos + Desbloquear todos Total Agrega dados para comenzar d%1$d diff --git a/app/src/main/res/values-fr/strings.xml b/app/src/main/res/values-fr/strings.xml index cc1e1f4..808f984 100644 --- a/app/src/main/res/values-fr/strings.xml +++ b/app/src/main/res/values-fr/strings.xml @@ -31,6 +31,7 @@ Éditer Lancer Tout lancer + Débloquer tout Total Ajoutez des dés pour commencer d%1$d diff --git a/app/src/main/res/values-ja/strings.xml b/app/src/main/res/values-ja/strings.xml index ceed92f..8a21ea3 100644 --- a/app/src/main/res/values-ja/strings.xml +++ b/app/src/main/res/values-ja/strings.xml @@ -31,6 +31,7 @@ 編集 転がす すべて転がす + すべてのロックを解除 合計 サイコロを追加して始めます d%1$d