Display "Current Interval" on the 'Set Due Date' dialog #18028
Conversation
|
Important Maintainers: This PR contains Strings changes
|
| findViewById<MaterialTextView>(R.id.current_interval_text)!!.also { | ||
| // Current interval cannot be shown if multiple cards are selected | ||
| if (viewModel.cardCount == 1) { | ||
| val currentCard = getColUnsafe().getCard(cardIds[0]) |
There was a problem hiding this comment.
Try using withCol { isntead of getColUnsafe. You'll find some examples if you search around the repository
There was a problem hiding this comment.
Also, getting the interval should be in the viewModel
There was a problem hiding this comment.
Hi @BrayanDSO updated with withCol and moved the fetching of interval to viewmodel.
BrayanDSO
added
the
Needs Author Reply
AnkiDroid/src/main/java/com/ichi2/anki/scheduling/SetDueDateViewModel.kt
Outdated
Show resolved
Hide resolved
|
Hey @BrayanDSO and @ShridharGoel, thanks for the reviews! I've addressed your comments--please take a look. |
ShridharGoel
added
Needs Second Approval
There was a problem hiding this comment.
I'd appreciate if you could ensure it all fits in a single commit, and have the commit message we want in the history.
Also a small change in documentation, and all is good.
I'd appreciate some tests too. We already have unit tests for SetDueDate dialog and view model, so it should hopefully be little extra work.
| @@ -112,12 +114,31 @@ class SetDueDateViewModel : ViewModel() { | |||
| field = value | |||
| } | |||
|
|
|||
| val currentInterval = MutableStateFlow<Int?>(null) | |||
There was a problem hiding this comment.
Please add some documentation. In particular, stating that the value is the number of day, and that:
- the value is not-null if exactly one card is selected, which is the only case in which the view should display the interval
| } | ||
| } | ||
| } else { | ||
| currentInterval.value = null |
There was a problem hiding this comment.
If there's a need to set this to null, comment it. If not, remove it
| * The value is not-null if exactly one card is selected, which is the only case | ||
| * in which the view should display the interval. | ||
| */ | ||
| val currentInterval = MutableStateFlow<Int?>(null) |
There was a problem hiding this comment.
You can optionally typealias or use a value class for documentation within the type system
| viewModelScope.launch { | ||
| withCol { | ||
| getCard(cardIds.first()).let { | ||
| currentInterval.value = it.ivl |
There was a problem hiding this comment.
This should only be set if the card is in the review state
A learning/relearning/new card has the value 0 shown here
lukstbit
added
the
squash-merge
Purpose / Description
This PR enhances the 'Set Due Date' dialog by displaying the "Current Interval" when a single card is selected. This provides users with better context on the existing interval before setting a new due date. If multiple cards are selected, the interval is hidden, as displaying it would not be meaningful in that scenario.
Fixes
Approach
Adds a display of the current interval to the "Set Due Date" dialog when a single card is selected. If multiple cards are selected, the interval is hidden as it would not be meaningful.
How Has This Been Tested?
Checklist
Please, go through these checks before submitting the PR.