-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat(fields): allow insertion of 'special fields' with preview #20041
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.
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
base: main
Are you sure you want to change the base?
Conversation
|
Important Maintainers: This PR contains Strings changes
|
| val dialog = | ||
| AlertDialog.Builder(requireContext()).create { | ||
| title(R.string.card_template_editor_select_field) | ||
| negativeButton(R.string.dialog_cancel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe remove this?
3a6e9e3 to
e9cd55b
Compare
| <string name="basic_fields_tab_header" comment="Tab header for inserting non-special fields: {{Front}}">Basic</string> | ||
| <string name="special_fields_tab_header" comment="Tab header for inserting special fields: {{Deck}}">Special</string> | ||
| <string name="special_field_example_suffix"><![CDATA[: ‘<b>%1$s</b>’]]></string> | ||
| <string name="special_field_front_side_help">The front template content. Audio is not automatically played</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please nitpick
| <string name="import_error_resolution_select_file">Open the file using your device’s file browser app</string> | ||
|
|
||
| <!-- Special Fields --> | ||
| <string name="basic_fields_tab_header" comment="Tab header for inserting non-special fields: {{Front}}">Basic</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Especially this - I feel I'm naming a new concept
EDIT:
The following are all better than 'Basic': I think 'Fields' makes sense
- Fields
- Normal
- Regular
- Standard
|
Look nice! I think the example tags should be something simpler like "Tag1 Tag2". I prefer "Normal" over "Basic". |
e9cd55b to
33f7e3b
Compare
|
@ZornHadNoChoice The tags come from the note itself. I'd rather do this, over sending |
33f7e3b to
29902aa
Compare
29902aa to
6502dd1
Compare
6502dd1 to
e434d3f
Compare
criticalAY
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool and clean
| fun all(side: SingleCardSide) = | ||
| ALL.filter { field -> | ||
| when { | ||
| field == FrontSide && side == FRONT -> false | ||
| else -> true | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we pre-compute these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not worth it IMO. More code for nanoseconds of performance (and that's if the JIT doesn't convert it to a constant lookup)
| class InsertFieldDialogViewModel( | ||
| savedStateHandle: SavedStateHandle, | ||
| ) : ViewModel() { | ||
| var currentTab: Tab = Tab.BASIC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there chances it will be killed if the system kills the process to save memory, just want to be sure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to a flow
| fun String.ellipsize(maxLength: Int): String { | ||
| require(maxLength > 1) { "invalid length: $maxLength" } | ||
| if (this.length <= maxLength) return this | ||
| return this.take(maxLength - 1) + "…" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a big concern but what if there is a emoji at max limit as last char, it will be split into 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests and a BreakIterator, ChatGPT for the APIs, myself for the implementation
I will be extending this class to handle field filters and special fields * rendering the output is now the responsibility of `renderToTemplateTag()` rather than done in the CardTemplateEditor
This introduces Anki's 'Special Fields' in the ViewModel
and adds a 'side' argument, so `{{FrontSide}}` can optionally
be displayed
This does not implement the user interface
See: https://docs.ankiweb.net/templates/fields.html#special-fields
Improves discoverability of this feature vs needing to go through the manual Assisted-by: GPT-5.2: ViewPager2.updateHeight
This provides an explanation of all Special Fields to the user in the 'Insert field' dialog This also produces contextual help for all fields based on the context of the selected card/note (if any) Assisted-by: GPT-5.2 - learning about BreakIterator
e434d3f to
5ae31c3
Compare
Purpose / Description
Anki has discoverability problems. One of the features which is unclear to users without reading the manual 'Special Fields': fields which relate to the metadata of the card.
{{Subdeck}}for example lists the current deckThis Pull Request updates the 'insert field' dialog to allows a user to select special fields. If the user came from 'manage note types' then the note type & card template have an example preview. If the user came from the note editor, then fields are populated based on the note ID, and the selected card template.
Approach
InsertFieldDialog: firstly just listing the special fields in a listHow Has This Been Tested?
Pixel 9 Pro
added template (no ord)

API 34 Tablet Emulator
Learning
Checklist