Skip to content

feat(deck-description): 'Format as Markdown' support & various UX improvements #18748

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

Merged
merged 14 commits into from
Jul 21, 2025

Conversation

david-allison
Copy link
Member

@david-allison david-allison commented Jul 2, 2025

Purpose / Description

I was looking into #18528. While I was making improvements to the screen, I found that the 'Anki 2.1.41+ handling' checkbox upstream enabled markdown support (but disabled <img>), so I added support for it

  • Improved Deck Description UI/UX
    • Better unsaved changes handling
    • ⚠️ I could not easily hook 'click outside', so I disabled this so changes are not lost
    • Swapped 'save' text for an icon
    • Disabled icon if no changes available
  • Deck Description screen now supports markdown if Format as Markdown is enabled
  • Deck Description: Added 'Format as Markdown'
    • And a help page with help icon

Fixes

Approach

  • expose the md property of deck
  • use it when rendering
  • support modification of md via EditDeckDescriptionDialog
  • Standard UI/UX improvements to EditDeckDescriptionDialog

How Has This Been Tested?

new
image

older

Screenshot 2025-07-02 at 18 01 51 Screenshot 2025-07-02 at 18 01 31 Screenshot 2025-07-02 at 18 01 59

Learning (optional, can help others)

ankitects/anki@ded626f

Checklist

  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • 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
    • ✅ Zero issues

Copy link
Contributor

github-actions bot commented Jul 2, 2025

Important

Maintainers: This PR contains Strings changes

  1. Sync Translations before merging this PR and wait for the action to complete
  2. Review and merge the auto-generated PR in order to sync all user-submitted translations
  3. Sync Translations again and merge the PR so the huge automated string changes caused by merging this PR are by themselves and easy to review

@david-allison david-allison force-pushed the deck-description branch 4 times, most recently from df7a81b to c852c79 Compare July 2, 2025 20:29
BrayanDSO

This comment was marked as resolved.

@BrayanDSO BrayanDSO added the Needs Author Reply Waiting for a reply from the original author label Jul 3, 2025
@david-allison
Copy link
Member Author

david-allison commented Jul 3, 2025

Updated screenshots, added corners via MaterialAlertDialogBuilder.

Long string to prove that the checkbox remains on screen

@david-allison david-allison removed the Needs Author Reply Waiting for a reply from the original author label Jul 3, 2025
@david-allison david-allison requested a review from BrayanDSO July 3, 2025 20:33
Copy link
Member

@lukstbit lukstbit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dialog expands to occupy full width of the screen(most obvious in tablets).
The user input is not saved if the app goes in the background and then comes back.

I know that I'm one of the people that approved the initial code but after looking at this the last couple of days I would recommend that we use a standard dialog with Cancel/Save buttons at the bottom instead of the current toolbar actions approach(used probably because the dialog doesn't play too well with very big content but I think we could get round this somehow).

Also, I'm surprised that you didn't use a ViewModel.

currentDescription = getDescription()
override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
this.dialogView = layoutInflater.inflate(R.layout.dialog_deck_description, null)
return MaterialAlertDialogBuilder(requireContext())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seeing that MaterialAlertDialogBuilder doesn't produce an ui like AlertDialog does we should settle which one we use going forward.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lukstbit lukstbit added Needs Author Reply Waiting for a reply from the original author and removed Needs Review labels Jul 13, 2025
@david-allison david-allison force-pushed the deck-description branch 4 times, most recently from 71a3b4e to 064d2cc Compare July 13, 2025 22:17
@david-allison
Copy link
Member Author

@lukstbit I have handled all requests

Screenshot 2025-07-13 at 23 17 36

@david-allison david-allison force-pushed the deck-description branch 2 times, most recently from 53057d0 to 5ff75ea Compare July 14, 2025 02:11
@mikehardy
Copy link
Member

Thanks for giving it a look again - it does seem like most (all?) items were addressed and, as ever, we can always do followups

@mikehardy mikehardy added this pull request to the merge queue Jul 21, 2025
@mikehardy mikehardy removed the Review High Priority Request for high priority review label Jul 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jul 21, 2025
@mikehardy
Copy link
Member

@david-allison merge queue caught a quiet conflict with refactorings and order of merge...

e: file:///home/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/EditDeckDescriptionDialogViewModelTest.kt:30:28 Unresolved reference 'TestClass'.
e: file:///home/runner/work/Anki-Android/Anki-Android/AnkiDroid/src/test/java/com/ichi2/anki/dialogs/EditDeckDescriptionDialogViewModelTest.kt:175:9 Unresolved reference 'TestClass'.
gradle/actions: Writing build results to /home/runner/work/_temp/.gradle-actions/build-results/__run-1753116949796.json

@mikehardy mikehardy added the Needs Author Reply Waiting for a reply from the original author label Jul 21, 2025
* fix margins to be standard 24dp
* add icon for save
* add content description for 'close'

Fixes Issue 18528
* fixes saved state restoration
Note that this option has unintended consequences.

* `<img>` tags, are now stripped, although they work with the option
 disabled
* It does not work on old versions of Anki

https://redirect.github.com/ankitects/anki/commit/ded626f0b9685355e9856c352daf21c09382999e

Issue 18749
Adds rounded corners

Issue 18528
Requested in review, instead of icons

Issue 18749
Didn't look good on tablets
@david-allison david-allison enabled auto-merge July 21, 2025 17:18
@david-allison david-allison removed the Needs Author Reply Waiting for a reply from the original author label Jul 21, 2025
@david-allison
Copy link
Member Author

@david-allison david-allison added this pull request to the merge queue Jul 21, 2025
Merged via the queue into ankidroid:main with commit ebc33d3 Jul 21, 2025
10 checks passed
Copy link
Contributor

Maintainers: Please Sync Translations to produce a commit with only the automated changes from this PR.

Read more about updating strings on the wiki,

@github-actions github-actions bot removed the Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) label Jul 21, 2025
@github-actions github-actions bot added this to the 2.22 release milestone Jul 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Deck Description: Support "Anki 2.1.41+ handling" The Edit deck description pop-up design is outdated
4 participants