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.

Sign up for GitHub

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

Jump to bottom

Make Scan selected text lazy and add Scan text at selection #915

Merged
merged 6 commits into from
May 14, 2024
Merged

Make Scan selected text lazy and add Scan text at selection #915

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cb6f254
Add Scan selected text lazy
Kuuuube May 11, 2024
39c2c58
Remove extra spaces
Kuuuube May 11, 2024
141d9a5
Improve naming
Kuuuube May 11, 2024
2ee3f59
Use disallowExpandSelection instead of "lazy"
Kuuuube May 14, 2024
f70e187
Use "keyword arguments" for _scanSelectedText
Kuuuube May 14, 2024
87637d0
Revert Use "keyword arguments" for _scanSelectedText
Kuuuube May 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions ext/js/app/frontend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export class Frontend {

this._hotkeyHandler.registerActions([
['scanSelectedText', this._onActionScanSelectedText.bind(this)],
['scanTextAtSelection', this._onActionScanTextAtSelection.bind(this)],
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly if renaming is possible I would rather name the current scanSelectedText to scanTextGreedy and repurpose scanSelectedText for this new functionality. I'm not sure about the naming of "lazy" tbh

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought about naming them like that but I feel like "greedy" and "lazy" aren't as well known terms to end users.

scanSelectedText just scans the selected text, simple.

scanTextAtSelection scans the text starting at the selection and does whatever it wants inside or outside the selection.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah these are user facing strings 🤔

['scanTextAtCaret', this._onActionScanTextAtCaret.bind(this)]
]);
/* eslint-enable @stylistic/no-multi-spaces */
Expand Down Expand Up @@ -256,14 +257,21 @@ export class Frontend {
* @returns {void}
*/
_onActionScanSelectedText() {
void this._scanSelectedText(false);
void this._scanSelectedText(false, true);
Kuuuube marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* @returns {void}
*/
_onActionScanTextAtSelection() {
void this._scanSelectedText(false, false);
}

/**
* @returns {void}
*/
_onActionScanTextAtCaret() {
void this._scanSelectedText(true);
void this._scanSelectedText(true, false);
}

// API message handlers
Expand Down Expand Up @@ -919,12 +927,13 @@ export class Frontend {

/**
* @param {boolean} allowEmptyRange
* @param {boolean} disallowExpandSelection
* @returns {Promise<boolean>}
*/
async _scanSelectedText(allowEmptyRange) {
async _scanSelectedText(allowEmptyRange, disallowExpandSelection) {
const range = this._getFirstSelectionRange(allowEmptyRange);
if (range === null) { return false; }
const source = TextSourceRange.create(range);
const source = disallowExpandSelection ? TextSourceRange.createLazy(range) : TextSourceRange.create(range);
await this._textScanner.search(source, {focus: true, restoreSelection: true});
return true;
}
Expand Down
24 changes: 19 additions & 5 deletions ext/js/dom/text-source-range.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ export class TextSourceRange {
* @param {?DOMRect} cachedSourceRect A cached `DOMRect` representing the rect of the `imposterSourceElement`,
* which can be used after the imposter element is removed from the page.
* Must not be `null` if imposterElement is specified.
* @param {boolean} disallowExpandSelection
*/
constructor(range, rangeStartOffset, content, imposterElement, imposterSourceElement, cachedRects, cachedSourceRect) {
constructor(range, rangeStartOffset, content, imposterElement, imposterSourceElement, cachedRects, cachedSourceRect, disallowExpandSelection) {
/** @type {Range} */
this._range = range;
/** @type {number} */
Expand All @@ -58,6 +59,8 @@ export class TextSourceRange {
this._cachedRects = cachedRects;
/** @type {?DOMRect} */
this._cachedSourceRect = cachedSourceRect;
/** @type {boolean} */
this._disallowExpandSelection = disallowExpandSelection;
}

/**
Expand Down Expand Up @@ -104,7 +107,8 @@ export class TextSourceRange {
this._imposterElement,
this._imposterSourceElement,
this._cachedRects,
this._cachedSourceRect
this._cachedSourceRect,
this._disallowExpandSelection
);
}

Expand Down Expand Up @@ -145,7 +149,8 @@ export class TextSourceRange {
}
const state = new DOMTextScanner(node, offset, !layoutAwareScan, layoutAwareScan).seek(length);
this._range.setEnd(state.node, state.offset);
this._content = (fromEnd ? this._content + state.content : state.content);
const expandedContent = fromEnd ? this._content + state.content : state.content;
this._content = this._disallowExpandSelection ? expandedContent : this._content;
return length - state.remainder;
}

Expand Down Expand Up @@ -252,7 +257,16 @@ export class TextSourceRange {
* @returns {TextSourceRange} A new instance of the class corresponding to the range.
*/
static create(range) {
return new TextSourceRange(range, range.startOffset, range.toString(), null, null, null, null);
return new TextSourceRange(range, range.startOffset, range.toString(), null, null, null, null, true);
}

/**
* Creates a new instance for a given range without expanding the search.
* @param {Range} range The source range.
* @returns {TextSourceRange} A new instance of the class corresponding to the range.
*/
static createLazy(range) {
return new TextSourceRange(range, range.startOffset, range.toString(), null, null, null, null, false);
}

/**
Expand All @@ -265,7 +279,7 @@ export class TextSourceRange {
static createFromImposter(range, imposterElement, imposterSourceElement) {
const cachedRects = convertMultipleRectZoomCoordinates(range.getClientRects(), range.startContainer);
const cachedSourceRect = convertRectZoomCoordinates(imposterSourceElement.getBoundingClientRect(), imposterSourceElement);
return new TextSourceRange(range, range.startOffset, range.toString(), imposterElement, imposterSourceElement, cachedRects, cachedSourceRect);
return new TextSourceRange(range, range.startOffset, range.toString(), imposterElement, imposterSourceElement, cachedRects, cachedSourceRect, true);
}

/**
Expand Down
1 change: 1 addition & 0 deletions ext/js/pages/settings/keyboard-shortcuts-controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export class KeyboardShortcutController {
['playAudioFromSource', {scopes: new Set(['popup', 'search']), argument: {template: 'hotkey-argument-audio-source', default: 'jpod101'}}],
['copyHostSelection', {scopes: new Set(['popup'])}],
['scanSelectedText', {scopes: new Set(['web'])}],
['scanTextAtSelection', {scopes: new Set(['web'])}],
['scanTextAtCaret', {scopes: new Set(['web'])}],
['toggleOption', {scopes: new Set(['popup', 'search']), argument: {template: 'hotkey-argument-setting-path', default: ''}}]
]);
Expand Down
1 change: 1 addition & 0 deletions ext/templates-settings.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,7 @@
<option value="playAudioFromSource">Play audio from source</option>
<option value="copyHostSelection">Copy host window selection</option>
<option value="scanSelectedText">Scan selected text</option>
<option value="scanTextAtSelection">Scan text at selection</option>
<option value="scanTextAtCaret">Scan text at caret</option>
<option value="toggleOption">Toggle option</option>
</select>
Expand Down
Loading