mortii / anki-morphs

A MorphMan fork rebuilt from the ground up with a focus on simplicity, performance, and a codebase with minimal technical debt.
https://mortii.github.io/anki-morphs/
Mozilla Public License 2.0
58 stars 9 forks source link

Add dropdown to find a highlighted word in am-morphs when studying #187

Closed xofm31 closed 6 months ago

xofm31 commented 6 months ago

When I am studying my sentence cards, I sometimes forget other morphs that aren't the one that I have the definition for on that card. I would like to be able to highlight the morph on the card, and have it find the card that has that highlighted text as the am-unknowns. I threw this together in the init.py, and it almost works. It does look for the highlighted text, but it does it before you get to the dropdown.

Hopefully you like this feature, or have a better suggestion on how to easily find the definition for a morph that is not the am-morphs on that card.

def search_am_unknowns_action(web_view: AnkiWebView, menu: QMenu) -> None:
    assert mw is not None
    selected_text = web_view.selectedText()
    if selected_text == "":
        return
    action = QAction(f'Find in {ankimorphs_globals.EXTRA_FIELD_UNKNOWNS}', menu)
    query = f'"{ankimorphs_globals.EXTRA_FIELD_UNKNOWNS}:{selected_text}"'
    browser = aqt.dialogs.open("Browser", mw)
    assert browser is not None

    search_edit: Optional[QLineEdit] = browser.form.searchEdit.lineEdit()
    assert search_edit is not None

    search_edit.setText(query)
    browser.onSearchActivated()
    menu.addAction(action)
mortii commented 6 months ago

Ah, you basically almost made the "browse same morph" feature you described in #181. The way it gets around the problem of having too many keyboard shortcuts is very clever, I like it!

It might need a little tweaking, but if you create a pull request we can discuss/work on it there, and we could possibly include it in the v1.4 update.

xofm31 commented 6 months ago

Ah, you basically almost made the "browse same morph" feature you described in https://github.com/mortii/anki-morphs/discussions/181.

It's not quite the same, because here, the purpose is to find card(s) that have already been learned, so no need to search for cards that have multiple unknowns. But yes, very similar.

It might need a little tweaking

You are very generous. It actually doesn't work. But I will create a pull request.

mortii commented 6 months ago

I've merged it into main: https://github.com/mortii/anki-morphs/commit/e0ccf25eddbbb4e1fc25144c6d73ddb925dd4a7e It will be included in the next update. Thanks!

mortii commented 6 months ago

@xofm31 any suggestions for where to place this in the guide?

xofm31 commented 6 months ago

Thank you for getting this in!

Since it is not a critical function, and its meaning is so clear, maybe you don't need to put it in the guide?

If you want to include it, I guess it would go under https://mortii.github.io/anki-morphs/user_guide/usage/reviewing-cards.html. In that case, I'd have a section for the dropdown & also include the marking a Name.

mortii commented 6 months ago

If you want to include it, I guess it would go under https://mortii.github.io/anki-morphs/user_guide/usage/reviewing-cards.html. In that case, I'd have a section for the dropdown & also include the marking a Name.

Great idea, I'll do that.

Btw, I just realized why I haven't seen dynamic labels in the context menu other places: wide

so, I'll revert them back to being static

xofm31 commented 6 months ago

I didn't think of that.

I don't have an opinion about the dynamic labels, but if you wanted to keep them, you could check the length of the highlighted text, and if it's more than a certain length (5? 10? 20?), you could either get rid of it, or shorten it. (Browse "am-unknowns:teach...").

mortii commented 6 months ago

I don't have an opinion about the dynamic labels, but if you wanted to keep them, you could check the length of the highlighted text, and if it's more than a certain length (5? 10? 20?), you could either get rid of it, or shorten it. (Browse "am-unknowns:teach...").

I thought about that, but clipping the text would make it lose much of its purpose, which was to display exactly the text that would be used, so I'll just go back to completely static.

github-actions[bot] commented 6 months ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.