From 934b98f83588263f32330047980fd1368266633e Mon Sep 17 00:00:00 2001 From: Jakub Fidler <31575114+RisingOrange@users.noreply.github.com> Date: Mon, 23 Dec 2024 18:53:11 +0100 Subject: [PATCH] [BUILD-939] Always use new chatbot button, fixes (#1061) * Hide sidebar config options behind mh_integration flag * Adjust on-hover colors for tabs * Remove old chatbot button * Fix flickering of reviewer buttons * Refactor * Only show chatbot button if feature flag is active * Update test_ankihub_ai_button * Fix test_ankihub_ai_token_is_set_when_token_is_saved * Refactor _get_enabled_buttons_list --- ankihub/gui/config_dialog.py | 14 +- ankihub/gui/reviewer.py | 95 ++++------ ankihub/gui/web/ankihub_ai_old.js | 281 ---------------------------- ankihub/gui/web/reviewer_buttons.js | 1 + ankihub/gui/web/sidebar_tabs.html | 4 +- ankihub/gui/web/templates.py | 3 +- tests/addon/test_integration.py | 39 ++-- 7 files changed, 75 insertions(+), 362 deletions(-) delete mode 100644 ankihub/gui/web/ankihub_ai_old.js diff --git a/ankihub/gui/config_dialog.py b/ankihub/gui/config_dialog.py index 5a3d5cc48..82b1baaa5 100644 --- a/ankihub/gui/config_dialog.py +++ b/ankihub/gui/config_dialog.py @@ -6,6 +6,7 @@ By doing the import inside the function we make sure that aqt.mw is set up when we import ankiaddonconfig.window during normal execution. (We access mw using aqt.mw across the codebase to prevent this problem.)""" + from typing import cast from ..settings import config @@ -52,12 +53,13 @@ def _general_tab(conf_window) -> None: tab.hseparator() tab.space(8) - tab.text("Sidebar", bold=True) - tab.checkbox("ankihub_ai_chatbot", "AnkiHub AI Chatbot") - tab.checkbox("boards_and_beyond", "Boards and Beyond") - tab.checkbox("first_aid_forward", "First Aid Forward") - tab.hseparator() - tab.space(8) + if config.get_feature_flags().get("mh_integration"): + tab.text("Sidebar", bold=True) + tab.checkbox("ankihub_ai_chatbot", "AnkiHub AI Chatbot") + tab.checkbox("boards_and_beyond", "Boards and Beyond") + tab.checkbox("first_aid_forward", "First Aid Forward") + tab.hseparator() + tab.space(8) tab.text("Debug", bold=True) tab.checkbox("report_errors", "Report errors") diff --git a/ankihub/gui/reviewer.py b/ankihub/gui/reviewer.py index 2851c0bd2..cc53fe8b5 100644 --- a/ankihub/gui/reviewer.py +++ b/ankihub/gui/reviewer.py @@ -299,7 +299,7 @@ def setup(): reviewer_did_show_question.append(_add_or_refresh_view_note_button) if not using_qt5(): - webview_will_set_content.append(_add_ankihub_ai_and_sidebar_and_buttons) + webview_will_set_content.append(_inject_ankihub_features_and_setup_sidebar) reviewer_did_show_question.append(_notify_ankihub_ai_of_card_change) reviewer_did_show_question.append(_notify_reviewer_buttons_of_card_change) reviewer_did_show_question.append(_notify_sidebar_of_card_change) @@ -370,71 +370,54 @@ def _add_or_refresh_view_note_button(card: Card) -> None: aqt.mw.reviewer.bottom.web.eval(js) -def _add_ankihub_ai_and_sidebar_and_buttons(web_content: WebContent, context): +def _inject_ankihub_features_and_setup_sidebar( + web_content: WebContent, context +) -> None: if not isinstance(context, Reviewer): return + reviewer_button_js = get_reviewer_buttons_js( + theme=_ankihub_theme(), + enabled_buttons=_get_enabled_buttons_list(), + ) + web_content.body += f"" + + ankihub_ai_js = get_ankihub_ai_js( + knox_token=config.token(), + app_url=config.app_url, + endpoint_path="ai/chatbot", + query_parameters="is_on_anki=true", + theme=_ankihub_theme(), + ) + web_content.body += f"" + + global reviewer_sidebar + if not reviewer_sidebar: + reviewer_sidebar = ReviewerSidebar(context) + reviewer_sidebar.set_on_auth_failure_hook(_handle_auth_failure) + + +def _get_enabled_buttons_list() -> List[str]: + buttons_map = {} + feature_flags = config.get_feature_flags() - # TODO This condition is not placed correctly. It should be used to - # show/hide the AnkiHub AI chatbot button when the reviewer_did_show_question hook is called. - # The consquence of the current implementation is that the chatbot icon is shown for notes it - # shouldn't be shown for and (maybe) isn't shown for notes it should be shown for in some cases. - # However, we don't have to fix it for the old chatbot implementation, because we will switch - # to a new one soon. For the new implementation, we should implement the correct logic. if feature_flags.get("chatbot"): - if feature_flags.get("mh_integration"): - ankihub_ai_js_template_name = "ankihub_ai.js" - else: - # The new chatbot js (ankihub_ai.js) doesn't show a button, so we can always set it up. - # However, the old chatbot js (ankihub_ai_old.js) shows a button when executed, so we only - # want to execute it when the note has note embeddings. - if not _related_ah_deck_has_note_embeddings(aqt.mw.reviewer.card.note()): - return - - ankihub_ai_js_template_name = "ankihub_ai_old.js" - - ankihub_ai_js = get_ankihub_ai_js( - template_name=ankihub_ai_js_template_name, - knox_token=config.token(), - app_url=config.app_url, - endpoint_path="ai/chatbot", - query_parameters="is_on_anki=true", - theme=_ankihub_theme(), - ) - web_content.body += f"" + buttons_map["ankihub_ai_chatbot"] = "chatbot" if feature_flags.get("mh_integration"): - global reviewer_sidebar - if not reviewer_sidebar: - reviewer_sidebar = ReviewerSidebar(context) - reviewer_sidebar.set_on_auth_failure_hook(_handle_auth_failure) - - reviewer_button_js = get_reviewer_buttons_js( - theme=_ankihub_theme(), - enabled_buttons=_get_enabled_buttons_list(), + buttons_map.update( + { + "boards_and_beyond": "b&b", + "first_aid_forward": "fa4", + } ) - web_content.body += f"" - -def _get_enabled_buttons_list() -> List[str]: - buttons_map = { - "ankihub_ai_chatbot": "chatbot", - "boards_and_beyond": "b&b", - "first_aid_forward": "fa4", - } - public_config = config.public_config - buttons_config = dict( - filter( - lambda item: item[0] - in ["ankihub_ai_chatbot", "boards_and_beyond", "first_aid_forward"], - public_config.items(), - ) - ) - enabled_buttons_list = [ - buttons_map[key] for key, value in buttons_config.items() if value + return [ + buttons_map[key] + for key, value in config.public_config.items() + if key in buttons_map and value ] - return enabled_buttons_list def _related_ah_deck_has_note_embeddings(note: Note) -> bool: @@ -583,8 +566,6 @@ def _on_js_message(handled: Tuple[bool, Any], message: str, context: Any) -> Any js = _wrap_with_ankihubAI_check("ankihubAI.hideIframe();") context.web.eval(js) else: - # TODO load correct sidebar content (Boards&Beyond, First Aid or AnkiHub Chatbot) - # depending on the button that was toggled if is_active: reviewer_sidebar.open_sidebar() resource_type = ResourceType(button_name) diff --git a/ankihub/gui/web/ankihub_ai_old.js b/ankihub/gui/web/ankihub_ai_old.js deleted file mode 100644 index 80045ad1c..000000000 --- a/ankihub/gui/web/ankihub_ai_old.js +++ /dev/null @@ -1,281 +0,0 @@ - -class AnkiHubAI { - constructor() { - this.appUrl = "{{ APP_URL }}"; - this.endpointPath = "{{ ENDPOINT_PATH }}"; - this.queryParameters = "{{ QUERY_PARAMETERS }}" - this.embeddedAuthPath = "common/embedded-auth"; - - this.noteIdOfReviewerCard = null; // The note ID for which the card is currently being reviewed. - this.noteIdOfChatbot = null; // The note ID for which the chatbot page is currently loaded. - this.authenticated = false; - this.knoxToken = "{{ KNOX_TOKEN }}"; - this.iframeVisible = false; - this.theme = "{{ THEME }}"; - - this.setup(); - } - - setup() { - this.iframe = this.setupIframe(); - [this.button, this.tooltip] = this.setupIFrameToggleButton(); - - this.setupMessageListener(); - let updateIframeHeight = this.updateIframeHeight - let updateIframeWidth = this.updateIframeWidth - let iframe = this.iframe - window.parent.addEventListener('resize', function () { - if (iframe.style.display !== "none") { - updateIframeHeight(iframe, window.parent.innerHeight) - updateIframeWidth(iframe, window.parent.innerWidth) - } - }); - } - - setupMessageListener() { - window.addEventListener("message", (event) => { - if (event.origin !== this.appUrl) { - return; - } - - if (event.data === "Authentication failed") { - this.hideIframe(); - this.invalidateSessionAndPromptToLogin(); - } - - if (event.data.sendToPython) { - pycmd(event.data.message); - } - }); - } - - setupIframe() { - const iframe = document.createElement("iframe"); - iframe.id = "ankihub-ai-iframe"; - this.setIframeStyles(iframe, window.parent.innerHeight); - - iframe.onload = () => { - if (iframe.src && iframe.src.includes(this.embeddedAuthPath)) { - const message = { - token: this.knoxToken, - theme: this.theme, - } - iframe.contentWindow.postMessage(message, this.appUrl); - } - }; - document.body.appendChild(iframe); - return iframe; - } - - setupIFrameToggleButton() { - const button = document.createElement("button"); - button.id = "ankihub-ai-button"; - this.setButtonStyles(button) - button.onclick = () => { - if (!this.knoxToken) { - this.invalidateSessionAndPromptToLogin() - return; - } - if (!this.iframeVisible) { - this.maybeUpdateIframeSrc(); - this.showIframe(); - this.hideTooltip(); - } else { - this.hideIframe(); - this.showTooltip(); - } - }; - document.body.appendChild(button); - - const tooltip = document.createElement("div"); - tooltip.id = "ankihub-ai-tooltip"; - tooltip.innerHTML = "Learn more about this flashcard
topic or explore related cards."; - - const tooltipArrow = document.createElement("div"); - tooltipArrow.id = "ankihub-ai-tooltip-arrow"; - tooltip.appendChild(tooltipArrow); - - this.setTooltipAndTooltipArrowStyles(tooltip, tooltipArrow); - - button.addEventListener("mouseover", () => { - if (this.iframe.style.display === "none") { - tooltip.style.display = "block"; - } - }); - - button.addEventListener("mouseout", function () { - tooltip.style.display = "none"; - }); - - document.body.appendChild(tooltip); - - return [button, tooltip]; - } - - setTooltipAndTooltipArrowStyles(tooltip, tooltipArrow) { - tooltip.style.position = "absolute"; - tooltip.style.bottom = "13px"; - tooltip.style.right = "93px"; - tooltip.style.zIndex = "1000"; - tooltip.style.fontSize = "medium"; - tooltip.style.borderRadius = "5px"; - tooltip.style.textAlign = "center"; - tooltip.style.padding = "10px"; - tooltip.style.display = "none"; - - tooltipArrow.style.position = "absolute"; - tooltipArrow.style.top = "50%"; - tooltipArrow.style.right = "-6px"; - tooltipArrow.style.marginTop = "-4px"; - tooltipArrow.style.width = "0"; - tooltipArrow.style.height = "0"; - tooltipArrow.style.borderLeft = "6px solid"; - tooltipArrow.style.borderTop = "6px solid transparent"; - tooltipArrow.style.borderBottom = "6px solid transparent"; - - const style = document.createElement("style"); - style.innerHTML = ` - :root { - --neutral-200: #e5e5e5; - --neutral-800: #1f2937; - } - - #ankihub-ai-tooltip { - background-color: var(--neutral-800); - color: white; - } - - .night-mode #ankihub-ai-tooltip { - background-color: var(--neutral-200); - color: black; - } - - #ankihub-ai-tooltip-arrow { - border-color: var(--neutral-800); - color: var(--neutral-800); - } - - .night-mode #ankihub-ai-tooltip-arrow { - border-color: var(--neutral-200); - color: var(--neutral-200); - } - `; - document.head.appendChild(style); - } - - showTooltip() { - this.tooltip.style.display = "block"; - } - - hideTooltip() { - this.tooltip.style.display = "none"; - } - - invalidateSessionAndPromptToLogin() { - this.authenticated = false; - this.knoxToken = null; - this.noteIdOfChatbot = null; - pycmd("ankihub_invalid_auth_token"); - } - - showIframe() { - this.button.style.backgroundImage = "url('_chevron-down-solid.svg')"; - this.button.style.backgroundSize = "30%"; - this.iframe.style.display = "block"; - this.iframeVisible = true; - } - - hideIframe() { - this.button.style.backgroundImage = "url('/_robot_icon.svg')"; - this.button.style.backgroundSize = "cover"; - this.iframe.style.display = "none"; - this.iframeVisible = false; - } - - cardChanged(noteId) { - this.noteIdOfReviewerCard = noteId; - - if (this.iframeVisible) { - this.maybeUpdateIframeSrc(); - } - } - - setToken(token) { - this.knoxToken = token; - } - - maybeUpdateIframeSrc() { - if (this.noteIdOfChatbot === this.noteIdOfReviewerCard) { - // No need to reload the iframe. - // This prevents the iframe from reloading when the user reopens the chatbot on the same card. - return; - } - - const targetUrl = `${this.appUrl}/${this.endpointPath}/${this.noteIdOfReviewerCard}/?${this.queryParameters}`; - if (!this.authenticated) { - this.iframe.src = `${this.appUrl}/${this.embeddedAuthPath}/?next=${encodeURIComponent(targetUrl)}`; - this.authenticated = true; - } else { - this.iframe.src = targetUrl; - } - - this.noteIdOfChatbot = this.noteIdOfReviewerCard; - } - - setButtonStyles(button) { - button.style.width = "40px"; - button.style.height = "40px"; - button.style.boxSizing = "content-box"; - button.style.padding = "8px 10px 8px 10px"; - button.style.margin = "0px 4px 0px 4px"; - - button.style.position = "fixed"; - button.style.bottom = "13px"; - button.style.right = "15px"; - button.style.zIndex = "9999"; - - button.style.borderRadius = "100%"; - button.style.border = "none"; - - button.style.backgroundImage = "url('/_robot_icon.svg')"; - button.style.backgroundSize = "cover"; - button.style.backgroundPosition = "center"; - button.style.backgroundRepeat = "no-repeat"; - button.style.backgroundColor = "#4f46e5"; - - button.style.cursor = "pointer"; - } - - setIframeStyles(iframe, parentWindowHeight) { - iframe.style.display = "none" - - iframe.style.width = "100%"; - iframe.style.maxWidth = "700px"; - iframe.style.minWidth = "360px"; - - iframe.style.height = "100%"; - iframe.style.maxHeight = `${parentWindowHeight - 95}px`; - - iframe.style.position = "fixed" - iframe.style.bottom = "85px" - iframe.style.right = "20px" - - iframe.style.border = "none" - iframe.style.borderRadius = "10px" - - iframe.style.boxShadow = "10px 10px 40px 0px rgba(0, 0, 0, 0.25)" - - iframe.style.overflow = "hidden" - } - - updateIframeHeight(iframe, parentWindowHeight) { - iframe.style.maxHeight = `${parentWindowHeight - 95}px`; - } - - updateIframeWidth(iframe, parentWindowWidth) { - iframe.style.width = `${parentWindowWidth - 36}px`; - } - -} - -window.ankihubAI = new AnkiHubAI(); diff --git a/ankihub/gui/web/reviewer_buttons.js b/ankihub/gui/web/reviewer_buttons.js index 60639c1bf..3afe9f239 100644 --- a/ankihub/gui/web/reviewer_buttons.js +++ b/ankihub/gui/web/reviewer_buttons.js @@ -128,6 +128,7 @@ class AnkiHubReviewerButtons { this.elementsContainer.appendChild(buttonContainer); this.elementsContainer.appendChild(toggleButtonsButton); + this.elementsContainer.style.visibility = "hidden"; document.body.appendChild(this.elementsContainer); this.injectResourceCountIndicatorStylesheet(); if (this.buttonsData.length == 0) { diff --git a/ankihub/gui/web/sidebar_tabs.html b/ankihub/gui/web/sidebar_tabs.html index f8aad29f1..a43178639 100644 --- a/ankihub/gui/web/sidebar_tabs.html +++ b/ankihub/gui/web/sidebar_tabs.html @@ -89,8 +89,8 @@ } .dark .webview-tab:hover { - color: #9CA3AF; - border-bottom: 2px solid #6B7280; + color: #D1D5DB; + border-bottom: 2px solid #4B5563 } .webview-tab.selected { diff --git a/ankihub/gui/web/templates.py b/ankihub/gui/web/templates.py index 1928ed1bd..1980c3af9 100644 --- a/ankihub/gui/web/templates.py +++ b/ankihub/gui/web/templates.py @@ -24,14 +24,13 @@ def get_header_webview_html( def get_ankihub_ai_js( - template_name: str, knox_token: str, app_url: str, endpoint_path: str, query_parameters: str, theme: str, ) -> str: - return env.get_template(template_name).render( + return env.get_template("ankihub_ai.js").render( { "KNOX_TOKEN": knox_token, "APP_URL": app_url, diff --git a/tests/addon/test_integration.py b/tests/addon/test_integration.py index 009794f89..bb9ebf3b0 100644 --- a/tests/addon/test_integration.py +++ b/tests/addon/test_integration.py @@ -6023,15 +6023,15 @@ def mock_using_qt5_to_return_false(mocker: MockerFixture): class TestAnkiHubAIInReviewer: @pytest.mark.sequential @pytest.mark.parametrize( - "feature_flag_active, has_note_embeddings, expected_button_exists", + "feature_flag_active, has_note_embeddings, expected_button_visible", [ - # The feature is only available for the AnKing deck and only if the feature flag is active + # The feature is only available for decks with note embeddings and only if the feature flag is active (True, True, True), (True, False, False), (False, True, False), ], ) - def test_basic( + def test_ankihub_ai_button( self, anki_session_with_addon_data: AnkiSession, import_ah_note: ImportAHNote, @@ -6040,31 +6040,31 @@ def test_basic( set_feature_flag_state: SetFeatureFlagState, feature_flag_active: bool, has_note_embeddings: bool, - expected_button_exists: bool, + expected_button_visible: bool, ): set_feature_flag_state("chatbot", feature_flag_active) entry_point.run() with anki_session_with_addon_data.profile_loaded(): + self._setup_token_and_app_url() + self._setup_note_for_review( install_ah_deck=install_ah_deck, import_ah_note=import_ah_note, has_note_embeddings=has_note_embeddings, ) - # Open reviewer aqt.mw.reviewer.show() qtbot.wait(300) assert not self._ankihub_ai_is_visible(qtbot) - ankihub_ai_button_exists = self._ankihub_ai_button_exist(qtbot) - assert ankihub_ai_button_exists == expected_button_exists - if not expected_button_exists: + assert self._ankihub_ai_button_visible(qtbot) == expected_button_visible + if not expected_button_visible: return - aqt.mw.reviewer.web.eval("ankihubAI.showIframe()") - qtbot.wait(300) + self._click_ankihub_ai_button() + qtbot.wait(500) assert self._ankihub_ai_is_visible(qtbot) @@ -6318,7 +6318,7 @@ def test_ankihub_ai_token_is_set_when_token_is_saved( qtbot.wait_until(lambda: self._ankihubAI_token(qtbot) == test_token) else: qtbot.wait(300) - assert self._ankihubAI_token(qtbot) is None + assert self._ankihubAI_token(qtbot) == "None" def _suspend_notes_by_ah_nids(self, ah_nids: List[uuid.UUID]): """Suspend all cards of the given notes for the given AnkiHub note ids.""" @@ -6331,6 +6331,12 @@ def _suspend_notes_by_ah_nids(self, ah_nids: List[uuid.UUID]): cards.append(card) aqt.mw.col.update_cards(cards) + def _setup_token_and_app_url(self) -> None: + config.save_token("test_token") + + # Prevent JS from making requests to the webapp + config.app_url = "http://localhost:3000" + def _setup_note_for_review( self, install_ah_deck: InstallAHDeck, @@ -6365,13 +6371,18 @@ def _ankihubAI_token(self, qtbot: QtBot) -> bool: ) return callback.args[0] - def _ankihub_ai_button_exist(self, qtbot) -> bool: + def _ankihub_ai_button_visible(self, qtbot) -> bool: with qtbot.wait_callback() as callback: aqt.mw.reviewer.web.evalWithCallback( - "document.getElementById('ankihub-ai-button')", + "document.getElementById('ankihub-chatbot-button-container').style.display !== 'none'", callback, ) - return callback.args[0] is not None + return bool(callback.args[0]) + + def _click_ankihub_ai_button(self) -> None: + aqt.mw.reviewer.web.eval( + "document.getElementById('ankihub-chatbot-button').click()" + ) class TestMaybeSendDailyReviewSummaries: