Skip to content
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

Add a hook for adding links to the top toolbar #448

Merged
merged 11 commits into from
Feb 21, 2020

Conversation

glutanimate
Copy link
Contributor

@glutanimate glutanimate commented Feb 15, 2020

Allows extending the top toolbar in Anki's main screen with additional links and/or modifying the existing ones.

Sample add-on (updated):

from aqt import gui_hooks
from aqt.utils import tooltip


def on_my_link_clicked():
    tooltip("My link clicked!")


def on_top_toolbar_did_init_links(links, toolbar):
    links.append(
        toolbar.create_link(
            "my-link", "My link", on_my_link_clicked, tip="My tooltip", id="my-link"
        )
    )


gui_hooks.top_toolbar_did_init_links.append(on_top_toolbar_did_init_links)

Allows extending the links in the top toolbar, in a similar
fashion to editor_did_init_shortcuts
Similar to aqt.editor.Editor.addButton
@dae
Copy link
Member

dae commented Feb 15, 2020

Tuples don't make for a very nice API - perhaps it's worth biting the bullet and breaking compat here by switching to a dataclass instead?

@dae
Copy link
Member

dae commented Feb 15, 2020

(please hold off on further changes to this hook for now - this may need a bit more thought, and some more changes should drop in the next few days that will need to be taken into consideration)

@glutanimate
Copy link
Contributor Author

Tuples don't make for a very nice API - perhaps it's worth biting the bullet and breaking compat here by switching to a dataclass instead?

Yeah, I think that would make sense. Doing a quick search through other add-ons' code bases, it seems like only MorphMan and Customize Keyboard Shortcuts currently patch Toolbar._centerLinks, so add-on breakage should be fairly small.

please hold off on further changes to this hook for now

Sure, will do! Does this also apply to hooks in other places? I was thinking of adding some hooks to deckbrowser.__renderPage and overview._renderPage, in the spirit of the changes in #258 that would have made it easier to add HTML to various parts of each view's _body (e.g. to prevent conflicts between add-ons like More Overview Stats and others that modify the overview page).

Either way, looking forward to the changes!

@dae
Copy link
Member

dae commented Feb 15, 2020

Just the top bar - those other places should be fine.

@dae
Copy link
Member

dae commented Feb 17, 2020

Ok, beta 1 is now out. The sync link won't fit in a simple three element dataclass, but perhaps the list could be a union of that dataclass or a string that is used verbatim?

@glutanimate
Copy link
Contributor Author

glutanimate commented Feb 20, 2020

Thanks! I ended up going with a slightly different approach (similar to editor_did_init_buttons). My thinking was that it would be beneficial to grant add-on authors a bit more flexibility in the types of elements they want to add to the toolbar, so I thought it would be best to just pass on a list of HTML strings.

Sorry about the extra merge commit and 075a279, BTW. For whatever reason my venv had an older version of pylib/tools installed as a package (???), and so the generated gui_hooks.py didn't contain to the changes you recently added to legacy hook handling.

@glutanimate
Copy link
Contributor Author

glutanimate commented Feb 20, 2020

Hmm, added links look a bit awkward now that there's the sync icon in between:

Screenshot_20200220_172042

Not sure how to best tweak this, though.

ToolbarLink was more of a vestigial left-over from an interim
implementation. This change simplifies link addition and brings
it closer in line with adding buttons in the editor screen
@glutanimate
Copy link
Contributor Author

Simplified the link creation a bit by doing away with the need for ToolbarLink.

@dae
Copy link
Member

dae commented Feb 21, 2020

Thanks!

Re your comments on the beta testing thread, I think Sync might be best kept as text for now, as the previous icon was not clear to some users. I don't have strong objections to it being moved to the right hand side again - feel free to send me a patch if you'd like to do that.

@dae dae merged commit b358550 into ankitects:master Feb 21, 2020
@glutanimate
Copy link
Contributor Author

Thanks for the merge!

I explored a few options regarding the toolbar, and posted about them in the beta testing thread. Though the gist of it is: None of them ended up being particularly convincing to me :/

@yiufung
Copy link

yiufung commented Mar 19, 2020

Hmm, added links look a bit awkward now that there's the sync icon in between:

Screenshot_20200220_172042

Not sure how to best tweak this, though.

Came from kaegi/MorphMan#97, and notice the extra underline in between too. When I click "Sync", the popup dialog is always shown up and is blocking, so I don't actually look at that icon frequently. I concur with @dae that it might be best kept as text.

Thanks for working on this patch too, the new interface makes adding links very handy.

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

Successfully merging this pull request may close these issues.

3 participants