Skip to content
This repository has been archived by the owner on Feb 25, 2023. It is now read-only.

[Feature Request] Make “View added note” button open Edit card dialog, not Browse #2070

Closed
oakkitten opened this issue Feb 14, 2022 · 14 comments

Comments

@oakkitten
Copy link
Contributor

I use the button “View added note” a lot to add finishing touches to my cards. This opens the browser. Some parts of the browser, such as the sidebar and the note list, take a lot of space and cannot be collapsed fully. I suggest adding an option to open Edit card dialog instead.

I imagine that the “Edit card” dialog, which doesn't exist at the moment, should look just like the existing “Edit Current” dialog, perhaps sans the “Close” button. This would require changes in AnkiConnect, and perhaps in Anki itself. I'm opening an issue here as if this doesn't get a green light, there's no point in asking for changes elsewhere.

@oakkitten
Copy link
Contributor Author

oakkitten commented Feb 19, 2022

For fun, I went and made the (proof of concept) Edit card dialog. I removed the close button, and added the Preview button as it appears in the Browser, and the Browse button that opens what Yomichan normally opens.
image
Works pretty great!

The code for the dialog
import aqt
from aqt import gui_hooks
from aqt.qt import QDialog, Qt, QDialogButtonBox, QKeySequence, QShortcut
from aqt.utils import disable_help_button, restoreGeom, saveGeom, tr

from aqt.browser import Browser
from aqt.browser.previewer import Previewer, MultiCardPreviewer, BrowserPreviewer
from aqt.utils import tooltip


class SimplePreviewer(MultiCardPreviewer):
    def __init__(self, mw, card_ids):
        super().__init__(parent=None, mw=mw, on_close=lambda: None)
        self.card_ids = card_ids
        self.current = 0
        self.last = 0

    def card(self):
        card_id = self.card_ids[self.current]
        return self.mw.col.get_card(card_id)

    def card_changed(self):
        changed = self.current != self.last
        self.last = self.current
        return changed

    def _on_prev_card(self) -> None:
        if self._should_enable_prev():
            self.current -= 1
        self.render_card()

    def _on_next_card(self) -> None:
        if self._should_enable_next():
            self.current += 1
        self.render_card()

    def _should_enable_prev(self) -> bool:
        return self.current > 0

    def _should_enable_next(self) -> bool:
        return self.current < len(self.card_ids) - 1

    def _render_scheduled(self) -> None:
        super()._render_scheduled()
        self._updateButtons()


class Edit(aqt.editcurrent.EditCurrent):
    def __init__(self, mw, note_id):
        QDialog.__init__(self, None, Qt.Window)
        mw.garbage_collect_on_dialog_finish(self)
        self.mw = mw
        self.form = aqt.forms.editcurrent.Ui_Dialog()
        self.form.setupUi(self)
        self.form.buttonBox.setVisible(False)   # hide the Close button bar
        self.setWindowTitle("Edit")
        disable_help_button(self)

        self.setup_editor()
        self.reopen(mw, note_id)

        self.setMinimumHeight(400)
        self.setMinimumWidth(250)
        restoreGeom(self, "edit")

        gui_hooks.operation_did_execute.append(self.on_operation_did_execute)

        self.show()

    def reopen(self, mw, note_id) -> None:
        if note_id:
            self.note_id = note_id
            self.editor.set_note(mw.col.get_note(note_id))

    def cleanup_and_close(self) -> None:
        gui_hooks.operation_did_execute.remove(self.on_operation_did_execute)
        self.editor.cleanup()
        saveGeom(self, "edit")
        aqt.dialogs.markClosed("Edit")
        QDialog.reject(self)

    ##########################################################################

    def show_browser(self, editor=None):
        aqt.dialogs.open("Browser", self.mw, 
                search=(f"nid:{self.note_id}",))

    def show_preview(self):
        card_ids = self.mw.col.find_cards(f"nid:{self.note_id}")
        if not card_ids:
            tooltip("no cards found")
        else:
            SimplePreviewer(self.mw, card_ids).open()

    ##########################################################################

    # adds browser button on the right-hand side for simplicity and visibility
    # adds preview button in its regular position with its regular shortcut
    def setup_editor(self):
        QShortcut(QKeySequence("Ctrl+Shift+P"), self, self.show_preview)
        QShortcut(QKeySequence("Ctrl+Shift+W"), self, self.show_browser)

        gui_hooks.editor_did_init.append(self.add_preview_button)
        gui_hooks.editor_did_init_buttons.append(self.add_browse_button)
        self.editor = aqt.editor.Editor(self.mw, self.form.fieldsArea, self)
        gui_hooks.editor_did_init.remove(self.add_preview_button)
        gui_hooks.editor_did_init_buttons.remove(self.add_browse_button)

    def add_browse_button(self, buttons, editor):
        button = editor.addButton(None, cmd='browse_', func=self.show_browser, 
                label="&nbsp;&nbsp;Browse&nbsp;&nbsp;")
        buttons.append(button)

    # from browser.py's setupEditor
    # copying _links is needed so that opening Anki's browser does not
    # screw them up as they are apparently shared between instances
    def add_preview_button(self, editor) -> None:
        editor._links = editor._links.copy()
        editor._links["preview"] = lambda _editor: self.show_preview()
        editor.web.eval(
            "$editorToolbar.then(({ notetypeButtons }) => "
            "notetypeButtons.appendButton("
            "{ component: editorToolbar.PreviewButton, id: 'preview' }"
            "));"
        )


aqt.dialogs.register_dialog("Edit", Edit)
Changes to AnkiConnect needed to use it For the proof of concept, I simply made AnkiConnect open this Edit dialog instead of the Browser.

The above code registers the Edit dialog on import. To use it, you can place it in a file edit.py in the AnkiConnect folder, and then change the method guiBrowse() in __init__.py to this:

@util.api()
def guiBrowse(self, query=None):
    if query is not None and query.startswith("nid:"):
        from . import edit
        aqt.dialogs.open("Edit", aqt.mw, int(query[4:]))
    else:
        # original contents of the method

@toasted-nutbread
Copy link
Collaborator

toasted-nutbread commented Feb 20, 2022

This could probably be added as a configuration option to Yomichan, or as some sort click modifier like shift+click, but I think backwards compatibility would be the primary factor here. Since you already have some base code, if you want to create a PR on https://github.com/FooSoft/anki-connect for changes, I can provide some pointers:

AnkiConnect API changes

The function here:
https://github.com/FooSoft/anki-connect/blob/a5aecfceee9652407bcb8e36c7cdc09199c2c952/plugin/__init__.py#L1349
Would probably need to be changed to something like:

def guiBrowse(self, query=None, options=None):
    if isinstance(options, dict) and 'editNoteId' in options:
        # Your changes, but using options['editNoteId']
        from . import edit
        aqt.dialogs.open("Edit", aqt.mw, int(options['editNoteId']))
        return []
    # Remainder of original contents

(Test this, I haven't tested this myself)

This would provide backwards compatibility, and forwards compatibility would fall-back to using the old behavior.

AnkiConnect README changes

Update https://github.com/FooSoft/anki-connect/blob/master/README.md#graphical-actions with info about the new options parameter.

Yomichan changes

Subsequently, the changes in Yomichan's anki.js would be something like this:

// ...
return await this._invoke('guiBrowse', {
    query: `nid:${noteId}`,
    options: {
        editNoteId: noteId
    }
});

There would be other changes to UI/settings, but that should all come after AnkiConnect actually supports the UI.

So IMO I would start by creating a PR on AnkiConnect and let @FooSoft hash out the implementation details on there, then proceed to determine the Yomichan implementation.

@FooSoft
Copy link
Owner

FooSoft commented Feb 20, 2022

I don't see any need to overload the existing guiBrowse action, why not just add something new? You are not actually "browsing" anything here, but editing a specific note, right?

@oakkitten
Copy link
Contributor Author

Yeah I was thinking of a new API. guiEditNote?

I could try making a PR but the thing is, while i made this work on my machine and on the current stable version of Anki, I'm just not sure how fragile is the code posted above. Anki's browser previewer is strongly tied to the browser so I made a new one but I had to use private methods; I added the preview button by copying stuff from browser.py but using any id other than preview makes it stop working and i'm not sure why, and I didn't try avoiding any memory leaks or stale timers or anything else i'm probably not aware about. this could potentially break as Anki updates or conflict with other plugin. i just don't know much about Anki internals.

p.s. the POC uses Ctrl+Shift+W as a shortcut for the browser because Ctrl+Shift+B doesn't work and—again—i have no idea why :p

@FooSoft
Copy link
Owner

FooSoft commented Feb 20, 2022

That's the reality of AnkiConnect... It's not using any public APIs, just poking at the internals. Things break over time, important things get fixed sooner rather than later.

@toasted-nutbread
Copy link
Collaborator

toasted-nutbread commented Feb 21, 2022

I don't see any need to overload the existing guiBrowse action, why not just add something new? You are not actually "browsing" anything here, but editing a specific note, right?

I guess my thought was to make compatibility as easy as possible. If someone has a new version of Yomichan which uses the new guiEditNote function, but they also have an older version of AnkiConnect which doesn't have guiEditNote, the method would do nothing. Does AnkiConnect have any method of API discovery to determine some function is available? If it does, that's maybe a way around this issue, although it makes it a bit more complicated than doing guiBrowse(query, {editNoteId: noteId}, which would fallback to being handled as guiBrowse(query) on older AnkiConenct versions.

@FooSoft
Copy link
Owner

FooSoft commented Feb 21, 2022

There's no good way to see if an action is available in AnkiConnect. That being said, you can just try calling the API and seeing if it errors out on the action not being available. This doesn't introduce any perceivable delay to the user.

@oakkitten
Copy link
Contributor Author

So... Edit dialog has now been merged into Anki-Connect (here's the API method). I find the dialog very satisfying, if I do say so myself. How about an option in Yomichan to use it? I could try making a PR but JavaScript is not my forte 🙏

I imagine a drop-down list with an option to either use Browser or this dialog. I'm not sure if something like Shift-clicking for the alternative would be useful, as you can always open Browser from the Edit dialog.

(Or just switch everyone to the Edit dialog. Who wants the ugly bulky browser? Ugh!)

@toasted-nutbread
Copy link
Collaborator

See #2143 for example. This is not complete, but it should allow testing.

First thing I notice about the AnkiConnect plugin is that the guiEditNote window doesn't attempt to take focus when the API is invoked. Contrast that with guiBrowseNote, which does focus the window (or flash the taskbar on Windows if it's not allowed to take focus).

@oakkitten
Copy link
Contributor Author

Thanks for bringing it up! Window activation is behaving very erratically in Qt, or maybe it's Windows to blame. I think I have figured out some of it at least now. (Somehow opening PyCharm screws it up, too! Took some time to catch that.)

I have a fix that works for me on my machine. (I'm running Windows 10, and am not using the registry hack.) In fact, it works even better than Browser does! Can you perhaps try it and see if it works for you as well? Only edit.py was changed, so you can just copy it into your Anki-Connect addon source folder to test.

Barring that, in the comment above the fix, I listed the cases in which the window wasn't properly activated. Can you confirm that this is what you saw? If you saw the window not being activated in other circumstances, could you please describe them?

@toasted-nutbread
Copy link
Collaborator

Windows has a bunch of restrictions related to when windows are allowed to become the foreground window, see this link for some details. This is probably because Windows has had a checkered past with malware taking advantage of APIs like this and being a nuisance, so Windows has had to lock them down quite a bit. That being said, if a window isn't allowed to become the foreground window, it usually will flash its tab on the taskbar.

Testing your latest update looks more consistent with the behaviour of the how the old GUI browser is opened (taskbar flashes or becomes foreground window).

I might suggest adding a link to your comment in edit.py to the SetForegroundWindow page, since I believe that's largely what causes all of the "odd" behaviour.

@oakkitten
Copy link
Contributor Author

By the way, Ubuntu does the same, only instead of flashing the taskbar it shows a toast on the top of the screen saying “Window is ready”. Pretty awful.

Anyway, I call the behavior erratic since, well, it's inconsistent and doesn't work as advertised. The behavior depends on stuff like whether PyCharm was opened since boot or not, whether the main window is minimized or not, which of the Anki windows gets focus last. I think the windows are supposed to never “steal focus” from Yomichan, but they sometimes do, both on Ubuntu and Windows. I suppose the issue is with Qt, it has had a lot of issues with window activation. (Opened one myself the other week!) Or perhaps the moon wobble was especially strong at that moment. I just don't see how it can be explained with the comment in SetForegroundWindow alone.

It might be worth getting down to the real issue, but for the time being I suppose it should be good enough that Edit dialog behaves no worse than Browser with regards to activation. But yeah, a few links won't hurt.

As for #2143, yeah I can test on my machine, thanks. I suppose this needs an option, though?
I could try making a PR with that, I guess, if you want me to. Not sure if I should, as you opened a draft already 😬

@toasted-nutbread
Copy link
Collaborator

toasted-nutbread commented May 20, 2022

I suppose it should be good enough that Edit dialog behaves no worse than Browser with regards to activation.

Yep, this is what I was looking for. I don't expect us to fix issues that aren't directly related to how the OS handles things, so just making the two consistent is sufficient.

#2143 is updated now with a setting.

@toasted-nutbread
Copy link
Collaborator

Resolved by #2143.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants