Open oakkitten opened 2 years ago
Though I have not yet thoroughly read and understood the whole thing, I like it as a whole. Thank you for your dedication to the work.
However, I am concerned about the icons of Card State. I maybe understand your intention to represent each development states of studying cards, but still it seems to be a leap of association to link "learning and review" to "young boy and matured man". Plus, Anki has already used the metaphor of growth in the classification of review cards ("Young" and "Mature"). It seems to be confusing to liken learning cards to young boy. Besides, I think it would be better to avoid gender-unneutral design as far as possible. Not a few people might resist the combination of review card and matured man (or matured woman) and feel it intrusive.
I don't want to use colors because colored icons don't look very well and because the association between the color of the circles in the sidebar and the numbers in the deck browser is just too weak in my opinion. Yes, the colors are the same but I don't feel that my mind jumps to “yes, this means ‘new’” when I see a blue circle.
I sympathize with you to some extent, but even if the colored icons aren't perfect, I think it would be better to adopt them for the above reasons and also from the perspective of design consistency with Statistics page, the card counts in the other pages, and Anki Desktop.
Ah, very valid points. I never thought about young/mature stuff. I dunno.
On one hand, I think these icons look awesome (if I do say so myself) and make a clear progression, I think. Also, they are naturally round, which fits with the circle that represents “state” in Anki. A circle is a good as any representation of a state, so I wanted to stick with it for consistency.
On the other hand, as you are saying there are concepts of young and mature already. The thing about all of those concepts, it's so hard to think of suitable icons for them, especially for learning. Colors are a suitable alternative, but I just hate them so much. If I was to go with colour I'd want color non-filled circles for new/learning/review and current icons for suspended/buried, I don't think these should be colored. But I still don't like this. I dunno.
As for gender neutrality, I'm absolutely for inclusivity but I just don't see how this could be an issue. I only chose moustache since how would you even depict a “mature lady” on an icon?
I think these icons look awesome
Yes, they really look awesome. I applaud your talent and skills for creating icon. I hope you will continue to make use of them for AnkiDroid and help us.
As for gender neutrality, I'm absolutely for inclusivity but I just don't see how this could be an issue. I only chose moustache since how would you even depict a “mature lady” on an icon?
As you say, it is not so much an issue. By no means do I intend to say that male icon is a male-dominated or sexist design. I just think that gender-neutral icon like most of material icons would be more desirable since it would be more simple and universal (Yeah, that's easier said than done. I do not mean to belittle your dedication and the works. Please don't have any hard feelings). Perhaps, gender-neutral maturity might be expressed by elevating the position of eyes and adding vertical line of bridge of nose...?
TLDR (quite busy here), so I'll comment only what caught my attention
Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically
This is a brief summary of my attempt to bring in chips to Browser. These chips behave similarly to chips in Gmail and replicate most of the functionality of the sidebar in Anki.
The implementation is here, see the few last commits. It's currently not PR quality, is presumably blocked by converting the card list to
RecyclerView
and might be split into several PRs.I would love it if someone took a brief look at
WizardAdapter
andCheckableTreeAdapter
and told me whether they like the general design of the thing.Behavior
A single-line chip bar, horizontally scrollable, is placed just below the toolbar. Tapping on a chip opens the according bottom sheet, where you can select one or multiple items. When you select items, search is instantly re-done, and the chip changes its color and label to reflect the selection. For example,
Decks
may change toCool deck
if one deck is selected orCool deck+1
if two are selected. In the latter case,Cool deck
would be the one the user selected first.The toolbar, along with the chip bar and the spinners bellow, is collapsed when the user scrolls down the list of cards.
The bottom sheets show lists of items with icons, text and checkboxes. To select items, you can:
All the bottom sheets have an item such as
All decks
without a checkbox. Tapping on it clears the selection and closes the dialog, effectively stopping filtering by this category.The bottom sheets can display directory tree-like items. Items that have subitems can be collapsed to hide those and show a clickable chevron to indicate collapsed state (
›
/⌄
). If there ara any chevrons, all items are shifted slightly to the right to make place for the leftmost chevrons.If there are many items, a search input is visible. The hint says
Search decks
, orSearch tags
, etc. Tapping on it will expand the sheet to full screen (if enough items). Search input has a clear search button that appears when there is some text. When typing, items that do not contain the input are hidden, with the exception of any parent items, e.g., searching forfoo
would changeThe bottom sheet survives rotation, preserving selection & scroll position.
Going forward
I would love to have a list of recent searches, and for the saved searches be more accessible. I imagine this could be done by not showing cards when opening the browser, but instead of showing something like this:
Caveats & considerations
I am not entirely sure about the “quick way to only select one thing” behavior. I like it myself, but maybe some user can find it confusing, Maybe simply toggle the checkbox when user taps on the label?
Different categories (Decks, Tags) are searched with
AND
, and items within categories withOR
. That is, if you select two decks and two tags, the search will be(deck1 OR deck2) AND (tag1 OR tag2)
. I think this is very not confusing. However, by clicking with Ctrl and Alt modifiers, Anki allows adding a single term with anAND
and negating it. That is, if you select one deck an Ctrl-Alt click on another one, you will getdeck1 AND NOT deck2
.I don't see a way for our interface to support that. How should the checkboxes look in such case, and what should the chip say? However, in perspective we might offer long-tapping on bottom sheet items, which would show a menu that would allow adding the terms with negation and whatnot into the search field, bypassing the chips.
Selecting a tag in the bottom sheet will filter by a tag but will not add
tag:tag1
to the input field. This behavior is very fitting for tags, but filtering byAgain
means a search forrated:1:1
, and this search item has parameters that the user might want to change. Current design does not allow that. This can be remedied via the above.Following this lively questionnaire, I think it would be nice to rename
Today
toTime
, makeDue
find all due cards, and removeReschedule
&Overdue
.In deck list there's no
Current deck
because why?Visuals
Pretty much what you can see from the screenshots. Some notes:
I think the chip bar along with the spinners below them can have the background color of the toolbar.
The chips can have transparent background which makes them behave just like Gmail chips in the sense that when you tap on them, the ripple effect only happens to the outline. I am not sure that this is the intended effect by Google, but I like how it feels. Can be changed to a more usual behavior.
When changing the items in the bottom sheet, default
RecyclerView
animations are run with the exception of the chevron animation. Instead of the default crossfade, when item's subitems are collapsed or shown, chevron is rotated 90°.I created some new icons that you can see in the screenshots.
marked
has a star icon.Text gravity depends on layout RTL regardless of text itself. Stuff like menus work this way.
Caveats & considerations
I'm yet to figure out the roundness of the corners of the bottom sheet but honestly no roundness looks OK too.
When the contents of the bottom sheet change height, the bottom sheet height change is not animated. Not sure what would be a good way to solve this.
In light mode, the bottom sheet may expand to the status bar as well. In some circumstances, due to what I think is a bug in the material library, the status bar icons fail to change their color to the dark ones. While this is annoying, the circumstances are rather rare so this is not a super big deal, and this problem will be masked when we change the color of the status bar to a light one.
In dark mode, the bottom sheet does not expand to the status bar for some reason.
Normally, the bottom sheet height is limited by its contents. It would be nice if when the user taps on the search input, the sheet would switch temporarily drop this limit, and keep the search input on top of the screen even if the list has few items. While the bottom sheet API should allow just that, the result is super buggy in many ways. Not sure if there's an easy workaround.
Implementation
At the moment, the implementation is minimally wired into the app, which is to say I did the absolute minimum amound of changes in the existing code to make it work. The new code is like this:
SearchParameters
is a parcelable data class that stores user input and all the filtering information. It makesString
queries that can be passed to the backend.Chips.kt
has functions that set up and update chips based on changes insearchParameters
.BottomSheetDialogFragmentFix
is a fix for an issue inBottomSheetDialogFragment
; in landscape, the peek height of it is too small.PieceOfSheetFragment
(hehe) which is a subclass of ↑ is a base for all bottom sheets. It inflates the layout (with aRecyclerView
, and with or without the search input) and wires together the views. Its subclasses are responsible to setting up the adapter and reacting tofilter changesclicks, etc. Edit: moved filtering to adapterWizardAdapter
is a universal modularRecyclerView
adapter with stable IDs that has what I named wizards. A wizard is a like a sub-adapter; it is responsible for binding some of the data to the view, with or without animation, and it knows how to react to clicks, etc. By picking and choosing various wizards, you can compose a new adapter that has behavior from all wizards. A wizard:WizardAdapter
properties of items it can animate;ViewHolderBuddy
object that is attached to everyViewHolder
. A buddybind()
s the properties that the wizard is aware of to the view, or if the change is within what the wizard can animate,animate()
s them.Edit: added
TreeFilteringWitch
; a witch is a component that deals withWizardAdapter.Item
s like a wizard, but does not participate in view actions.TreeAdapter
(to be renamed?)CheckableTreeAdapter
is a final subclass ofWizardAdapter
that selects a few wizards for use with the bottom sheets.Currently it is also responsible for hiding/showing collapsing items; whileCollapseWizard
can animate the chevon and knows which items are collapsed, it can't alter the list itself (to be changed?).CheckableTreeAdapter
accepts a list of items like this. This list is not a tree structure. It could be a tree structure, which is a more straightforward choice, but I think that we can get away with flat lists. Consider a list of tags:foo::bar::baz zoo foo
Let's call every string in this list “a chain”; a chain is a
::
separated string, and let's call what's between::
s a chain link. If we add missing subchains (foo::bar
), and case-insensitive sort, we get:foo foo::bar foo::bar::baz zoo
And this is something that we can already show! The
text
would be the leaf chain (foo
,bar
), theindent
would be the number of links in the chain.This is what
Iterable<Row>.toCheckableSourceItems()
and the code around it does; it takes a bunch of rows (to be renamed, row is a chain with the associated ID and icon) and converts them toCheckableTreeAdapter.SourceItem
. So the path of transformation of data would be like this:Row
s with IDs and icons(filtered)list ofCheckableTreeAdapter.SourceItem
sCheckableTreeAdapter.SourceItem
to a list ofWizardAdapter.Item
, ready to be shownAll containers in question are (for the purpose of this) deeply immutable.
Caveats & considerations
At the moment the design is not yet finalized, and I'm not even sure if some the ideas are good. Having a modular adapter seems nice but modularity drastically increases code size; collapsing
CheckableTreeAdapter
andWizardAdapter
into a single class halves the code. But being modular should allow reusing the adapter in other places of the app, such as deck picker, etc. The above path from a set of chains to the adapter items is perhaps a bit too confusing and can be simplified. Some strings need to be extracted.If the card list has few items, dragging the empty area does not collaps/uncollapse the toolbar. This is probably a problem with the
ListView
. Wrapping it inNestedScrollView
doesn't seem to work. I hope that changing it to aRecyclerView
will resolve the issue.I didn't implement a chip with note types because I was lazy and because I'm not sure it's useful.
Currently, bottom sheet fragments are aware of the collection and that's where these get the data in sync. I think they should be able to get that data cached. This is simple and this ensures that the adapter has the latest data. However, this means that on view recreation the data can change. For the most part this is not an issue, but tags are strings and don't have intrinsic IDs, and tag position in tag list is used instead. In theory, if for instance the tags change between activity recreations, you can have
RecyclerView
restored to a wrong position. I don't think this can lead to any important issues, but just to be safe the IDs can be somewhat stabilized by keeping a static map of tag to id, or interning strings maybe. (Is string interning fast?)The collapse state of items is not saved and vanishes on activity recreation, and I don't think it's worth saving it.
The new icons should be optimized.
Code reuse
If deemed good, the
WizardAdapter
may serve in other parts of the app, such as the deck picker, or notification preferences. It will have to be finalized first, and some of the code should be generalized more yet, e.g., currently the chain collection → deck bottom sheet is tied to bottom sheets and must be untied. This is not hard but is hard to do right. Also, if the bottom sheets are to be reused in different activities, these should be united fromCardBrowser
.The
WizardAdapter
& pals may be an independent PR.