feat: Show cards preview (PR #2121)

Related issues

Proposed changes

Describe the big picture of your changes here to communicate to the reviewers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.

How to reproduce

Explain like if someone who doesn’t know this project is reviewing your changes and how they can replicate this.

Screenshots and videos

Paste here any screenshots or videos related to your changes, if applicable.

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc…

GitHub

Codecov Report

Attention: Patch coverage is 39.13043% with 56 lines in your changes are missing coverage. Please review.

Project coverage is 31.39%. Comparing base (73d3a73) to head (db25b85).

Additional details and impacted files
@@             Coverage Diff             @@
##              dev    #2121       +/-   ##
===========================================
- Coverage   97.81%   31.39%   -66.42%     
===========================================
  Files         156      155        -1     
  Lines       21734    21814       +80     
===========================================
- Hits        21259     6849    -14410     
- Misses        475    14965    +14490     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

@cjasm @heitorado @abdnh @RisingOrange This is the initial version of the card preview feature that I had been working on. Please feel free to provide any feedback on how it can be enhanced or if there are any misunderstandings.

This is very cool!

Some thoughts:

  • I like the grid of notes, the preview button and the flipping of the cards!
  • The preview is currently not using the CSS of the note type.
  • Not all note type features are supported right now, for example:
  • Since the preview strips all javascript, some note types that rely on js won’t show up correctly.
    • Maybe we can just say that it’s the responsibility of the deck maintainers to create note types that are presentable without javascript. However users can’t update note types on AnkiHub yet.

I think it’s fine to not support all features that Anki has when rendering cards, as well as not supporting javascript. The preview on AnkiHub could be a lite version of how it’s rendered in Anki.
We need to make sure that the AnKing note types show up in a presentable way. (They use various scripts and note type features.)

:wrench: If there are multiple cloze deletions with the same number (e.g. multiple {{c1::}} clozes), they produce one card, not multiple cards.
The logic also doesn’t consider multiple templates. One note type can have multiple templates. For example, the “Basic (and reversed card)”, which is built-in into Anki, has two templates.

:question: This is not used right now?

Really nice!

  • We are going to use the grid of notes and the preview button (eye button) as the new deck details page now? Or we are going until a design handoff is made? I’m just wondering if this PR exists as a reference so we can reuse the code in our current project or if the intention is to merge it.
  • We could reuse the note_cards.html fragment just by calling the decks:get-note-cards, right? This is nice because it will help us to implement the gallery view/card hover. I wonder if more adjustments are necessary or if is just calling the URL?

The preview is currently not using the CSS of the note type.

Yeah, I’m not using it because I couldn’t make the browser apply the style, maybe I’m missing something.

Not all note type features are supported right now, for example:
mathjax: Math & Symbols - Anki Manual
special fields: Field Replacements - Anki Manual
hint fields: Field Replacements - Anki Manual

You are right, since these features are more complex I decided not to implement that for now

Since the preview strips all javascript, some note types that rely on js won’t show up correctly.
Maybe we can just say that it’s the responsibility of the deck maintainers to create note types that are presentable without javascript. However users can’t update note types on AnkiHub yet.

I have to strip that for security reasons and because some js use objects/functions available only on the anki environment which will not work on ankihub.

yeah, I didn’t find a way to apply the CSS style in the card rendering

If there are multiple cloze deletions with the same number (e.g. multiple {{c1::}} clozes), they produce one card, not multiple cards.

I made a commit 76bc001f83126263f1d06508dea0bc8d0fcdbc41 fixing that.

The logic also doesn’t consider multiple templates. One note type can have multiple templates. For example, the “Basic (and reversed card)”, which is built-in into Anki, has two templates.

I noticed that, but I didn’t find a way to handle that, I also don’t know how anki chooses which one will render when clicking on the “preview” button. Do you have some idea of how to handle that?

We are going to use the grid of notes and the preview button (eye button) as the new deck details page now? Or we are going until a design handoff is made? I’m just wondering if this PR exists as a reference so we can reuse the code in our current project or if the intention is to merge it.

I don’t know. I created this to try a different approach to show the notes on the UI, but most of this interface was created only for testing the feature itself. That’s the reason why I didn’t create a migration for the feature flag, for now, all the changes made are for testing purposes.

We could reuse the note_cards.html fragment just by calling the decks:get-note-cards, right? This is nice because it will help us to implement the gallery view/card hover. I wonder if more adjustments are necessary or if is just calling the URL?

We have to make more adjustments like how to show the list of cards and how to prevent showing some images that is still blocked.

Considering the adjustments, this is pretty much what we expected for that feature.

Related to the NoteTypes, the most important is replicating Anki’s default and then supporting Anking ones. Agreeing with Jakub, we can manage Anking NoteTypes as nice to have. Other NoteTypes are definitely no-gos.

We are going to use the grid of notes and the preview button (eye button) as the new deck details page now? Or we are going until a design handoff is made? I’m just wondering if this PR exists as a reference so we can reuse the code in our current project or if the intention is to merge it.

I don’t think we are planning to change the deck preview page, but we can use that grid as the idea for the grid on chatbot answer to display the list of notes for the sources container

It seems like we can just add this:

        template = f"{template}\n<style>{self.cards_css}</style>"

after this:
https://github.com/ankipalace/ankihub/blob/8f9eb6e59b21f962c92ce9ed49ce8458f4a5fdaa/ankihub/decks/models.py#L862

this worked, but it’s interfering with the page’s style

I think we could use shadow DOM for that: Using shadow DOM - Web APIs | MDN
It seems a bit tricky to work with, but it isolates CSS.

I made a rough proof of concept:

https://github.com/ankipalace/ankihub/assets/31575114/bba66c1e-9c14-41f4-8566-66babad6a013

I added this to the template:
"\n<style>div, #div { background: blue; }</style>"
(I’m using #div here because the divs on the note type use “div” as an id.)

When the shadow DOM is not used, the style applies globally.
When the shadow DOM is used, only the divs inside the shadow DOM get the blue background.

https://github.com/ankipalace/ankihub/pull/2140

how anki chooses which one will render when clicking on the “preview” button

Seems like it just uses the first one.

I didn’t find a way to handle that

I have some ideas on how to handle that, but it might be better to work on this later. I think we might only need one card per note for the preview in the AnkiHub AI feature.

I know how to handle multiple cards, what I don’t know is how to handle multiple templates

I don’t think we are planning to change the deck preview page

I think it would be good to show this to the designers and see what they think about it. This or something similar could be a nice improvement.

This is nice, I only had heard about shadow DOM but never used it before

Yes, I understood it this way.

It’s kinda complicated.
It generates cards for each template and then puts them all together.
For non-cloze note types it’s one card per template.
For cloze note types it’s how many different cloze indexes there are.

But then there is also a rule that when the front side of the card would be empty, it’s not created.
And the front side can be empty sometimes and sometimes not, depending on conditional replacements.