Fix example text popup not updating correctly when scan resolution is set to word
#1527
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1526
The html for the example text upon loading the settings page
In a normal operations, when a user clicks on the example text, the
<span>
becomes hidden and the<input>
becomes visible. However, all search is done through the value set in the<span>
class.We have the class
PopupPreviewFrame
(plus an obscure css rule) that manages the state between the<input>
and the<span>
elements.When search is triggered, Yomitan searches for value inside
<span>
. Since the span is hidden while editing the example text, the element would normally be skipped to scanning in DOMTextScanner. But since the initial node of the text source range IS the span text, the first node value in DOMTextScanner is processed no matter whatIn the case of the
to determine we've reached a word boundary. HOWEVER, this causes a search that starts outside the
scanResolution
set toword
, upon initiating the scan, we try to expand the beginning of the initial range to encompass the entire word by looking backwards to find the word boundary. In the case of example text, we exit the<span>
text to find a spacevisibility: none
<span>
element to be skipped due to howDOMTextScanner
skips hidden elements.Why didn't this break in other UX flows?
In the case of a
scanResolution = 'character'
, we don't try to change the initial range startOffset, so the initial starting node for the search is within the hidden<span>
In the case of a
scanResolution = 'word'
page load, the<input>
value is hidden while<span>
is visible, so moving the startOffset of the initial range doesn't affect the scanability of the span text.Solutions considered
So in terms of fixes I had two potential options that I've explored. The first is to fix the behavior of the visibility of
<input>
and<span>
to update correctly BEFORE the search begins since. This might be weird UX though since I'm not sure what would happen when you update the visibility of an input element while still in focus.The second potential fix is to prevent the DOMTextScanner from leaving the
<span>
element. I tried implementing this earlier and it does complicate the DOMTextScanner a bit by forcing the scanner to peek at the contents of the previous DOM node before traversing it, which I feel like violates some abstraction boundaries of how DOMTextScanner is currently implemented.Solution proposed
I figured the easiest solution is to just add an extra option
disallowExpandStartOffset
that is set totrue
only in the example text popup frame. This prevents the startOffset from leaving the initial hidden<span>
, thereby allowing scan to successfully work. This fixes our problem "cleanly" without changing too much of how we manage the popup-preview or the DOMTextScanner