[Feat]: Word-Level Navigation for Text-Heavy HTML in Live Preview #2244
Conversation
Based on customer request to jump directly to individual words in Live Preview rather than just sections.
|
|
Thanks for the contributing @atondapu To clarify the licensing situation, we'd like to request your explicit permission to distribute the changes you've submitted in this pull request. Can you do one of the following to help me continue with this change?
|
There was a problem hiding this comment.
Thanks for this feature! I have left a few comments mainly to help me understand how this is implemented for reviewing.
Also how was this tested(In desktop app/browser)? Did you run the test suite?
I tested the code out and its working pretty well. But putting an extra button for this may not be the best UX discover-ability wise.
One way i think is more natural is to remove the text location button, and if the user clicks on an element that is not already selected, we select that element. and if an element is already selected, we do the word selection logic. This is similar to double click to edit text workflow(or click again on a selected element to locate text).
| @@ -3,6 +3,7 @@ | |||
| <div style="width: 20%;display: flex;"> | |||
| <button id="reloadLivePreviewButton" title="{{clickToReload}}" class="btn-alt-quiet toolbar-button reload-icon"></button> | |||
| <button id="highlightLPButton" title="{{toggleLiveHighlight}}" class="btn-alt-quiet toolbar-button pointer-fill-icon"></button> | |||
| <button id="wordNavigationLPButton" title="{{toggleWordNavigation}}" class="btn-alt-quiet toolbar-button text-icon"></button> | |||
There was a problem hiding this comment.
We tend to avoid placing new buttons on the live preview unless its absolutely required to keep the UI clutter in check.
Since this can be the default behavior, please tie this to the already existing toggleLiveHighlight flag and remove the new button.
| @@ -389,6 +411,7 @@ define(function (require, exports, module) { | |||
| $iframe = $panel.find("#panel-live-preview-frame"); | |||
| $pinUrlBtn = $panel.find("#pinURLButton"); | |||
| $highlightBtn = $panel.find("#highlightLPButton"); | |||
| $wordNavigationBtn = $panel.find("#wordNavigationLPButton"); | |||
There was a problem hiding this comment.
We don't need a new button for this, we can remove all these code as it can be tied to toggleLiveHighlight button.
| @@ -31,6 +31,9 @@ | |||
| "file.previewHighlight": [ | |||
| "Ctrl-Shift-C" | |||
| ], | |||
| "file.previewWordNavigation": [ | |||
There was a problem hiding this comment.
We usually add shortcuts only if it is very frequently used. since people are not likely to toggle this feature, a shortcut isn't necessary.
| @@ -302,13 +315,22 @@ define(function main(require, exports, module) { | |||
| _updateHighlightCheckmark(); | |||
| }); | |||
|
|
|||
| PreferencesManager.stateManager.definePreference("livedevWordNavigation", "boolean", false) | |||
There was a problem hiding this comment.
reuse the livedevHighlight button that already exists for this flow.
| @@ -147,31 +147,335 @@ define(function (require, exports, module) { | |||
| } | |||
| } | |||
|
|
|||
| function _tagSelectedInLivePreview(tagId, nodeName, contentEditable, allSelectors) { | |||
| function _findWordInEditor(editor, word, nodeName, tagId, wordContext, wordOccurrenceIndex) { | |||
There was a problem hiding this comment.
Can you give a high level overview of how this works in plain text? It will help me understand the changes. Specifically:
- How we identify the text in the remote live preview.
- How it communicates this position and finds the correct text position in the editor.
| const nodeText = textNode.textContent; | ||
|
|
||
| // Function to extract a word and its occurrence index | ||
| function extractWordAndOccurrence(text, wordStart, wordEnd) { |
There was a problem hiding this comment.
what will happen if we don't do word search? is the text not not always the correct target on click?
| } | ||
|
|
||
| // If occurrence index search failed or no occurrence index available, try context as a fallback | ||
| if (wordContext) { |
There was a problem hiding this comment.
i see that there are a few fallbacks here, What cases triggers these fullbacks? A few examples can help me understand the edge cases better. We may also need to add live preview integration tests for those cases later.
| function getClickedWord(element, event) { | ||
|
|
||
| // Try to find the clicked position within the element | ||
| const range = document.caretRangeFromPoint(event.clientX, event.clientY); |
There was a problem hiding this comment.
This api doesn't work in Firefox, so maybe we can fallback to caretPositionFromPoint in Firefox(caretPositionFromPoint doesnt work in safari), or disable the feature in Firefox.
|
@atondapu This is a cool feature to have. Would you be able to check this out or need any assistance? |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
PR Type: Feature
#2239
Fixes #2239
Feature Description
This PR adds the ability to navigate directly to individual words in Live Preview rather than just sections. When enabled, clicking on a word in the Live Preview will position the editor cursor at the exact character offset of that word in the source code, making it easier to find and edit specific content in text-heavy documents.
Screenshots: Find attached
Breaking Change: No
Tests Done: Passes all listing, existing tests, underwent manual general and edge-case testing to ensure correct word is found when button is toggled. (works across different browsers, and OS as before)
Additional Information
The implementation extends the existing section-level click handler in Live Preview to calculate the exact word position. A new toggle button with a translucent document glyph allows users to switch between section-level and word-level navigation modes. Special care was taken to handle edge cases with duplicate words by using click coordinates to identify the precise text node and word instance.