feat: Edit tags of a multiple notes / deck #8751

TarekkMA · 2021-05-02T00:45:46Z

Based on #8766

Pull Request template

Purpose / Description

The final PR of the TagsDialog refactoring series :D,

Fixes

Approach

Tags that are checked in all selected notes will appear as checked, if some notes have a tag but others don't then the tags are classified as indeterminate.

`CheckBoxTriStates`

A tristate checkbox (Like this one in flutter) was needed to display the 3 states the tag can be (checked, unchecked, indeterminate).

I added a cycleBackToIndeterminate property on the tristate checkbox, so if the user clicked on the checkbox it doesn't cycle back to an indeterminate state.

Used icons are the provided material ones in android studio

`TagsUtil#getUpdatedTags`

The code used to extract the updated tags from (prev, selected, indeterminate) lists.

`TagsList`

is now aware of the concept of indeterminate tags (updated all tested)

`UpdateMultipleNotes`

new task for updating multiple notes

How Has This Been Tested?

Unit tests

Checklist

You have not changed whitespace unnecessarily (it makes diffs hard to read)
You have a descriptive commit message with a short title (first line, max 50 chars).
Your code follows the style of the project (e.g. never omit braces in if statements)
You have commented your code, particularly in hard-to-understand areas
You have performed a self-review of your own code
UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
UI Changes: You have tested your change using the Google Accessibility Scanner

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/tags/TagsDialog.java

AnkiDroid/src/main/java/com/ichi2/async/CollectionTask.java

david-allison

I really like this, thanks!

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/tags/TagsDialog.java

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/tags/TagsDialogListener.java

AnkiDroid/src/main/java/com/ichi2/async/CollectionTask.java

AnkiDroid/src/main/java/com/ichi2/ui/CheckBoxTriStates.java

AnkiDroid/src/main/java/com/ichi2/utils/TagsUtil.java

AnkiDroid/src/test/java/com/ichi2/anki/dialogs/tags/TagsListTest.java

AnkiDroid/src/main/java/com/ichi2/anki/CardBrowser.java

AnkiDroid/src/main/java/com/ichi2/ui/CheckBoxTriStates.java

TarekkMA · 2021-05-03T23:20:54Z

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/tags/TagsList.java

-            boolean lhsChecked = isChecked(lhs);
-            boolean rhsChecked = isChecked(rhs);
+            boolean lhsChecked = isChecked(lhs) || isIndeterminate(lhs);
+            boolean rhsChecked = isChecked(rhs) || isIndeterminate(rhs);

            if (lhsChecked != rhsChecked) {
                // checked tags must appear first


Right now indeterminate tags have the same priority as checked when sorting, should we make checked then indeterminate then unchecked?

TarekkMA · 2021-05-04T20:50:06Z

Added a test to test all tag variations are showing as expected.

mikehardy · 2021-06-27T14:25:14Z

@TarekkMA looks like this needs re-basing as we're unwinding your stack of chained PRs - if you have a moment (and I understand it might be exam times for you! no pressure, it's not urgent) could you re-push this ?

TarekkMA · 2021-06-28T09:51:39Z

I have just finished my exams, I will give this PR a look and repush.

mikehardy · 2021-11-05T02:22:06Z

@david-allison-1 what do you think on this one? The test cover is really good, and it's a feature that is highly requested. I'd like to merge this to rescue it from entropy and deliver a great feature

david-allison · 2021-11-05T08:18:07Z

@david-allison-1 what do you think on this one? The test cover is really good, and it's a feature that is highly requested. I'd like to merge this to rescue it from entropy and deliver a great feature

Can this wait until Sunday? I don't have the free time to give this an appropriate amount of review or testing. Ping me if I don't look at this by Monday

mikehardy · 2021-11-05T13:03:04Z

@david-allison-1 what do you think on this one? The test cover is really good, and it's a feature that is highly requested. I'd like to merge this to rescue it from entropy and deliver a great feature

Can this wait until Sunday? I don't have the free time to give this an appropriate amount of review or testing. Ping me if I don't look at this by Monday

Oh sure! Not a rush per se, just a statement of intention that I'd like it to not rot

this will be used to get updated tags in case there are indeterminate tags

onSelectedTags now returns list of `indeterminateTags`, the indeterminateTags should be ignored if not needed.

this should have no effect on normal call with only checked tags

based on TagsDialogListenerAction the listener callback will do different things for now the only action doable is FILTER action

now the name and comment reflects the meaning of the tags since we can add/remove tags from multiple notes using this dialog type

Select multiple cards and change the tags of their notes

mikehardy · 2021-11-12T14:10:01Z

I just rescued it from conflict again and re-pushed

Can this wait until Sunday? I don't have the free time to give this an appropriate amount of review or testing. Ping me if I don't look at this by Monday

Gentle nudge :-)

david-allison

Tested and this looks good

david-allison · 2021-11-13T11:25:54Z

AnkiDroid/src/test/java/com/ichi2/anki/dialogs/tags/TagsListTest.java

+                Arrays.asList("aA", "bb", "cc", "dd", "ff"), list.copyOfAllTagList());
+        assertEquals("Checked tags list should not contain any duplicates  (case insensitive)",
+                Arrays.asList("ff"), list.copyOfCheckedTagList());
+        assertEquals("Checked tags list should not contain any duplicates  (case insensitive)\n" +


This seems logically correct, but it's handled by the above assert

mikehardy · 2021-11-13T12:33:29Z

Okay, deep breath, here we go

mikehardy · 2021-12-12T01:57:30Z

Hi there! November OpenCollective Notice

Apologies for the delay, and if you're already submitted something to OpenCollective for PRs merged in November 2021, you may feel free to ignore this and it will be processed shortly

Just a friendly notice that we try to process OpenCollective payments monthly - it's time for November 2021 submissions

If you are interested in compensation for this work, the process with details is here:

https://github.com/ankidroid/Anki-Android/wiki/OpenCollective-Payment-Process#how-to-get-paid

(I only post one comment per person to avoid spamming you, regardless of the number of PRs merged, but this note applies to all PRs merged for in the month of August)

Thanks!

mikehardy · 2021-12-12T01:57:53Z

@TarekkMA !!! We finally landed this one ❤️

TarekkMA · 2021-12-12T08:42:59Z

This is awesome, thank you guys for your reviews and guidance.
Very proud to be a contributor to AnkiDroid ❤️.

TarekkMA commented May 2, 2021

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/dialogs/tags/TagsDialog.java Show resolved Hide resolved

TarekkMA force-pushed the multi-tag-edit branch 5 times, most recently from 0ee0b30 to 8f09a28 Compare May 2, 2021 21:15

TarekkMA marked this pull request as ready for review May 2, 2021 21:26

TarekkMA marked this pull request as draft May 2, 2021 21:36

TarekkMA force-pushed the multi-tag-edit branch from 8f09a28 to ed9a150 Compare May 2, 2021 22:51

TarekkMA marked this pull request as ready for review May 2, 2021 22:51

TarekkMA commented May 2, 2021

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/async/CollectionTask.java Outdated Show resolved Hide resolved

david-allison requested changes May 3, 2021

View reviewed changes

TarekkMA commented May 3, 2021

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/ui/CheckBoxTriStates.java Show resolved Hide resolved

TarekkMA force-pushed the multi-tag-edit branch 3 times, most recently from 93bce3d to 2969f52 Compare May 3, 2021 13:18

TarekkMA commented May 3, 2021

View reviewed changes

TarekkMA force-pushed the multi-tag-edit branch 5 times, most recently from dfeabe6 to 9e116bf Compare May 4, 2021 20:33

mikehardy added the Needs Author Reply Waiting for a reply from the original author label Jun 27, 2021

TarekkMA force-pushed the multi-tag-edit branch 2 times, most recently from f6ef727 to edcad4f Compare June 28, 2021 14:26

mikehardy added Needs Review and removed Needs Author Reply Waiting for a reply from the original author labels Jun 28, 2021

david-allison self-assigned this Nov 5, 2021

mikehardy mentioned this pull request Nov 8, 2021

Kotlin migration TagsDialog #9832

Merged

TarekkMA added 13 commits November 12, 2021 07:13

feat: add TagsUtil#getUpdatedTags

1c4b7cc

this will be used to get updated tags in case there are indeterminate tags

NF: change signature of TagsDialogListener#onSelectedTags

9c34f0e

onSelectedTags now returns list of `indeterminateTags`, the indeterminateTags should be ignored if not needed.

NF: rename currentTags to checkedTags

609fc0c

feat: support for indeterminate tags

2868554

feat: TagsDialog support for indeterminate tags

595edd9

this should have no effect on normal call with only checked tags

feat: Add CheckBoxTriStates

756d13c

feat: adapter support for indeterminate tags

bacb73c

NF: refactor listener to do multiple actions

d1bc4cc

based on TagsDialogListenerAction the listener callback will do different things for now the only action doable is FILTER action

NF: rename ADD_TAG to EDIT_TAGS

9a15109

now the name and comment reflects the meaning of the tags since we can add/remove tags from multiple notes using this dialog type

feat: add UpdateMultipleNotes task

9b81a1d

feat: add "Edit Tags" in CardBrowser

c0a2f25

Select multiple cards and change the tags of their notes

NF: make TagsListTest more readable

de7dbb7

test: test all tag states are showing correctly

ae75405

mikehardy force-pushed the multi-tag-edit branch from 97e8706 to ae75405 Compare November 12, 2021 14:03

david-allison approved these changes Nov 13, 2021

View reviewed changes

mikehardy merged commit f6525c2 into ankidroid:master Nov 13, 2021

mikehardy removed Review High Priority Request for high priority review Needs Second Approval Has one approval, one more approval to merge labels Nov 13, 2021

mikehardy added this to the 2.16 release milestone Nov 13, 2021

david-allison mentioned this pull request Jan 20, 2024

[Bug] In Edit tags window, hierarchical tags are not shown at the top #15278

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Edit tags of a multiple notes / deck #8751

feat: Edit tags of a multiple notes / deck #8751

TarekkMA commented May 2, 2021 •

edited

Loading

david-allison left a comment

TarekkMA May 3, 2021

TarekkMA commented May 4, 2021

mikehardy commented Jun 27, 2021

TarekkMA commented Jun 28, 2021

mikehardy commented Nov 5, 2021

david-allison commented Nov 5, 2021

mikehardy commented Nov 5, 2021

mikehardy commented Nov 12, 2021

david-allison left a comment

david-allison Nov 13, 2021

mikehardy commented Nov 13, 2021

mikehardy commented Dec 12, 2021

mikehardy commented Dec 12, 2021

TarekkMA commented Dec 12, 2021

feat: Edit tags of a multiple notes / deck #8751

feat: Edit tags of a multiple notes / deck #8751

Conversation

TarekkMA commented May 2, 2021 • edited Loading

Pull Request template

Purpose / Description

Fixes

Approach

CheckBoxTriStates

TagsUtil#getUpdatedTags

TagsList

UpdateMultipleNotes

How Has This Been Tested?

Checklist

david-allison left a comment

Choose a reason for hiding this comment

TarekkMA May 3, 2021

Choose a reason for hiding this comment

TarekkMA commented May 4, 2021

mikehardy commented Jun 27, 2021

TarekkMA commented Jun 28, 2021

mikehardy commented Nov 5, 2021

david-allison commented Nov 5, 2021

mikehardy commented Nov 5, 2021

mikehardy commented Nov 12, 2021

david-allison left a comment

Choose a reason for hiding this comment

david-allison Nov 13, 2021

Choose a reason for hiding this comment

mikehardy commented Nov 13, 2021

mikehardy commented Dec 12, 2021

mikehardy commented Dec 12, 2021

TarekkMA commented Dec 12, 2021

TarekkMA commented May 2, 2021 •

edited

Loading

`CheckBoxTriStates`

`TagsUtil#getUpdatedTags`

`TagsList`

`UpdateMultipleNotes`