From 4a43b41f79d46b8a677bbfb11c27d43e126e670f Mon Sep 17 00:00:00 2001 From: toasted-nutbread Date: Sat, 25 Jul 2020 09:58:06 -0400 Subject: [PATCH] Set content refactor (#686) * Simplify invoke * Pass isTerms instead of type * Update DisplaySearch.setContent to pass argument array * Simplify argument object structure for setContent * Move focus and disableHistory level * Always include focus and disableHistory options * Change disableHistory to history * Pass source text to setContent * Use consistent argument/object field order * Remove unused previous field * Combine logic for forward/back navigation --- ext/bg/js/search.js | 29 +++++++----- ext/fg/js/frontend.js | 20 +++++--- ext/fg/js/popup.js | 2 +- ext/mixed/js/display-context.js | 7 +-- ext/mixed/js/display.js | 84 ++++++++++++++++++--------------- 5 files changed, 83 insertions(+), 59 deletions(-) diff --git a/ext/bg/js/search.js b/ext/bg/js/search.js index b3e3ebca..93f45468 100644 --- a/ext/bg/js/search.js +++ b/ext/bg/js/search.js @@ -158,10 +158,10 @@ class DisplaySearch extends Display { } } - async setContent(type, details) { + async setContent(...args) { this._query.blur(); this._closePopups(); - return await super.setContent(type, details); + return await super.setContent(...args); } clearContent() { @@ -171,12 +171,14 @@ class DisplaySearch extends Display { // Private - _onQueryParserSearch({type, definitions, sentence, cause}) { - this.setContent(type, { + _onQueryParserSearch({type, definitions, sentence, cause, textSource}) { + this.setContent({ + focus: false, + history: cause !== 'mouse', + type, + source: textSource.text(), definitions, context: { - focus: false, - disableHistory: cause === 'mouse', sentence, url: window.location.href } @@ -254,12 +256,17 @@ class DisplaySearch extends Display { this._updateSearchButton(); if (valid) { const {definitions} = await api.termsFind(query, details, this.getOptionsContext()); - this.setContent('terms', {definitions, context: { + this.setContent({ focus: false, - disableHistory: true, - sentence: {text: query, offset: 0}, - url: window.location.href - }}); + history: false, + definitions, + source: query, + type: 'terms', + context: { + sentence: {text: query, offset: 0}, + url: window.location.href + } + }); } else { this.clearContent(); } diff --git a/ext/fg/js/frontend.js b/ext/fg/js/frontend.js index 49cf4ca6..73cea841 100644 --- a/ext/fg/js/frontend.js +++ b/ext/fg/js/frontend.js @@ -426,12 +426,21 @@ class Frontend { this._showPopupContent( textSource, optionsContext, - type, - {definitions, context: {sentence, url, focus, disableHistory: true}} + { + focus, + history: false, + type, + source: textSource.text(), + definitions, + context: { + sentence, + url + } + } ); } - _showPopupContent(textSource, optionsContext, type=null, details=null) { + _showPopupContent(textSource, optionsContext, details=null) { this._lastShowPromise = this._popup.showContent( { source: this._id, @@ -439,10 +448,7 @@ class Frontend { elementRect: textSource.getRect(), writingMode: textSource.getWritingMode() }, - { - type, - details - } + details ); this._lastShowPromise.catch((error) => { if (yomichan.isExtensionUnloaded) { return; } diff --git a/ext/fg/js/popup.js b/ext/fg/js/popup.js index 6f2f0a88..281913c6 100644 --- a/ext/fg/js/popup.js +++ b/ext/fg/js/popup.js @@ -146,7 +146,7 @@ class Popup { } if (displayDetails !== null) { - this._invokeApi('setContent', {type: displayDetails.type, details: displayDetails.details}); + this._invokeApi('setContent', {details: displayDetails}); } } diff --git a/ext/mixed/js/display-context.js b/ext/mixed/js/display-context.js index 5ee78459..2322974a 100644 --- a/ext/mixed/js/display-context.js +++ b/ext/mixed/js/display-context.js @@ -17,8 +17,9 @@ class DisplayContext { - constructor(type, definitions, context) { + constructor(type, source, definitions, context) { this.type = type; + this.source = source; this.definitions = definitions; this.context = context; } @@ -43,8 +44,8 @@ class DisplayContext { return this.context.next; } - static push(self, type, definitions, context) { - const newContext = new DisplayContext(type, definitions, context); + static push(self, type, source, definitions, context) { + const newContext = new DisplayContext(type, source, definitions, context); if (self !== null) { newContext.update({previous: self}); self.update({next: newContext}); diff --git a/ext/mixed/js/display.js b/ext/mixed/js/display.js index f681b671..b2d7d54d 100644 --- a/ext/mixed/js/display.js +++ b/ext/mixed/js/display.js @@ -202,19 +202,18 @@ class Display { } } - async setContent(type, details) { + async setContent(details) { const token = {}; // Unique identifier token this._setContentToken = token; try { this._mediaLoader.unloadAll(); - const {definitions, context, focus} = details; + const {focus, history, type, source, definitions, context} = details; - if (context.disableHistory) { - delete context.disableHistory; - this._context = new DisplayContext(type, definitions, context); + if (!history) { + this._context = new DisplayContext(type, source, definitions, context); } else { - this._context = DisplayContext.push(this._context, type, definitions, context); + this._context = DisplayContext.push(this._context, type, source, definitions, context); } if (focus !== false) { @@ -226,7 +225,7 @@ class Display { case 'kanji': { const {sentence, url, index=0, scroll=null} = context; - await this._setContentTermsOrKanji(type, definitions, sentence, url, index, scroll, token); + await this._setContentTermsOrKanji((type === 'terms'), definitions, sentence, url, index, scroll, token); } break; } @@ -335,8 +334,8 @@ class Display { this.setOptionsContext(optionsContext); } - _onMessageSetContent({type, details}) { - this.setContent(type, details); + _onMessageSetContent({details}) { + this.setContent(details); } _onMessageClearAutoPlayTimer() { @@ -378,8 +377,16 @@ class Display { url: this._context.get('url') }; - const definitions = await api.kanjiFind(link.textContent, this.getOptionsContext()); - this.setContent('kanji', {definitions, context}); + const source = link.textContent; + const definitions = await api.kanjiFind(source, this.getOptionsContext()); + this.setContent({ + focus: false, + history: true, + type: 'kanji', + source, + definitions, + context + }); } catch (error) { this.onError(error); } @@ -419,13 +426,18 @@ class Display { scroll: this._windowScroll.y }); const context = { - disableHistory: false, sentence, - url: this._context.get('url'), - previous: this._context + url: this._context.get('url') }; - this.setContent('terms', {definitions, context}); + this.setContent({ + focus: false, + history: true, + type: 'terms', + source: textSource.text(), + definitions, + context + }); } catch (error) { this.onError(error); } @@ -581,8 +593,7 @@ class Display { } } - async _setContentTermsOrKanji(type, definitions, sentence, url, index, scroll, token) { - const isTerms = (type === 'terms'); + async _setContentTermsOrKanji(isTerms, definitions, sentence, url, index, scroll, token) { this._setEventListenersActive(false); this._definitions = definitions; @@ -727,33 +738,32 @@ class Display { } _sourceTermView() { - if (!this._context || !this._context.previous) { return; } - this._context.update({ - index: this._index, - scroll: this._windowScroll.y - }); - const previousContext = this._context.previous; - previousContext.set('disableHistory', true); - const details = { - definitions: previousContext.definitions, - context: previousContext.context - }; - this.setContent(previousContext.type, details); + this._relativeTermView(false); } _nextTermView() { - if (!this._context || !this._context.next) { return; } + this._relativeTermView(true); + } + + _relativeTermView(next) { + if (this._context === null) { return false; } + + const relative = next ? this._context.next : this._context.previous; + if (!relative) { return false; } + this._context.update({ index: this._index, scroll: this._windowScroll.y }); - const nextContext = this._context.next; - nextContext.set('disableHistory', true); - const details = { - definitions: nextContext.definitions, - context: nextContext.context - }; - this.setContent(nextContext.type, details); + this.setContent({ + focus: false, + history: false, + type: relative.type, + source: relative.source, + definitions: relative.definitions, + context: relative.context + }); + return true; } _noteTryAdd(mode) {