Skip to content

Commit

Permalink
fix(ui): Make Google Doc Canvas checks more lenient (#962)
Browse files Browse the repository at this point in the history 
I found in offline mode I the cached Google Docs query selector was always false due to executing before page loaded. I now query the selector on every `mousemove` which seems fast enough. An improvement would be to memoize a function or perhaps add a on Dom loaded callback.
I also removed the check for if the target was `svg` or `rect` since there's at least one counter example to that assumption. For now let's avoid premature optimization.

I also had to update the tests to not overwrite the Google Docs class since it now needs to be present at each execution.

Fixes #897
Fixes #881
  • Loading branch information
@melink14
melink14 authored Apr 7, 2022
1 parent 0579f8e commit 6809154
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 42 deletions.
14 changes: 4 additions & 10 deletions extension/rikaicontent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,6 @@ class RcxContent {
private defaultDict = 2;
private nextDict = 3;

private isGoogleDocPage =
document.querySelector('.kix-canvas-tile-content') !== null;

// Adds the listeners and stuff.
enableTab(config: Config) {
if (window.rikaichan === undefined) {
Expand Down Expand Up @@ -900,7 +897,7 @@ class RcxContent {
// don't try to highlight google docs
if (
rp &&
!this.isGoogleDoc(rp) &&
!this.isGoogleDoc() &&
((tdata.config.highlight &&
!this.mDown &&
!('form' in tdata.prevTarget!)) ||
Expand Down Expand Up @@ -1154,7 +1151,7 @@ class RcxContent {
HTMLSelectElement;
// Put this in a try catch so that an exception here doesn't prevent editing due to div.
try {
if (this.isGoogleDoc(eventTarget)) {
if (this.isGoogleDoc()) {
gdocRect = this.getRectUnderMouse(ev);
if (gdocRect) {
fake = this.makeFake(gdocRect);
Expand Down Expand Up @@ -1395,11 +1392,8 @@ class RcxContent {
);
}

private isGoogleDoc(eventTarget: Element | CharacterData): boolean {
return (
this.isGoogleDocPage &&
(eventTarget.nodeName === 'svg' || eventTarget.nodeName === 'rect')
);
private isGoogleDoc(): boolean {
return document.querySelector('.kix-canvas-tile-content') !== null;
}
}

Expand Down
76 changes: 44 additions & 32 deletions extension/test/rikaicontent_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,12 @@ describe('RcxContent', function () {
});

describe('with Google Docs annotated canvas', function () {
let docCanvas: HTMLDivElement;
beforeEach(function () {
markDocumentWithGoogleDocsClass();
docCanvas = document.querySelector<HTMLDivElement>(
'.kix-canvas-tile-content'
)!;
// Reinitialize rcxContent now that special class is rendered.
initializeRcxContent();
});
Expand All @@ -332,10 +336,10 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);

triggerMousemoveAtElementStart(root.querySelector('svg')!);
triggerMousemoveAtElementStart(docCanvas.querySelector('svg')!);
// Tick the clock forward to account for the popup delay.
clock.tick(1);

Expand All @@ -358,10 +362,10 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);

triggerMousemoveAtElementCenter(root.querySelector('svg')!);
triggerMousemoveAtElementCenter(docCanvas.querySelector('svg')!);
// Tick the clock forward to account for the popup delay.
clock.tick(1);

Expand All @@ -384,9 +388,9 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);
const rect = root.querySelector('rect')!;
const rect = docCanvas.querySelector('rect')!;

simulant.fire(rect, 'mousemove', {
clientX: Math.ceil(rect.getBoundingClientRect().left - 1),
Expand Down Expand Up @@ -414,9 +418,9 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);
const rect = root.querySelector('rect')!;
const rect = docCanvas.querySelector('rect')!;

simulant.fire(rect, 'mousemove', {
clientX: Math.ceil(rect.getBoundingClientRect().left + 30),
Expand Down Expand Up @@ -444,9 +448,9 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);
const rect = root.querySelector('rect')!;
const rect = docCanvas.querySelector('rect')!;

simulant.fire(rect, 'mousemove', {
clientX: Math.ceil(rect.getBoundingClientRect().left),
Expand Down Expand Up @@ -474,9 +478,9 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);
const rect = root.querySelector('rect')!;
const rect = docCanvas.querySelector('rect')!;

simulant.fire(rect, 'mousemove', {
clientX: Math.ceil(rect.getBoundingClientRect().left),
Expand Down Expand Up @@ -518,9 +522,9 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);
const rect = root.querySelector('rect#startrect')!;
const rect = docCanvas.querySelector('rect#startrect')!;

simulant.fire(rect, 'mousemove', {
clientX: Math.ceil(rect.getBoundingClientRect().left - 1),
Expand Down Expand Up @@ -558,9 +562,9 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);
const rect = root.querySelector('rect#startrect')!;
const rect = docCanvas.querySelector('rect#startrect')!;

simulant.fire(rect, 'mousemove', {
clientX: Math.ceil(rect.getBoundingClientRect().left + 18),
Expand Down Expand Up @@ -598,9 +602,9 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);
const rect = root.querySelector('rect#startrect')!;
const rect = docCanvas.querySelector('rect#startrect')!;

simulant.fire(rect, 'mousemove', {
clientX: Math.ceil(rect.getBoundingClientRect().left),
Expand Down Expand Up @@ -638,9 +642,9 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);
const rect = root.querySelector('rect#startrect')!;
const rect = docCanvas.querySelector('rect#startrect')!;

simulant.fire(rect, 'mousemove', {
clientX: Math.ceil(rect.getBoundingClientRect().left),
Expand Down Expand Up @@ -669,10 +673,10 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);

triggerMousemoveAtElementStart(root.querySelector('svg')!);
triggerMousemoveAtElementStart(docCanvas.querySelector('svg')!);
// Tick the clock forward to account for the popup delay.
clock.tick(1);

Expand Down Expand Up @@ -710,10 +714,10 @@ describe('RcxContent', function () {
fill="rgba(0,0,0,0.15)"
></rect>
</svg>`,
root
docCanvas
);

triggerMousemoveAtElementStart(root.querySelector('rect#hlrect')!);
triggerMousemoveAtElementStart(docCanvas.querySelector('rect#hlrect')!);
// Tick the clock forward to account for the popup delay.
clock.tick(1);

Expand Down Expand Up @@ -749,10 +753,12 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);

triggerMousemoveAtElementStart(root.querySelector('rect#startrect')!);
triggerMousemoveAtElementStart(
docCanvas.querySelector('rect#startrect')!
);
// Tick the clock forward to account for the popup delay.
clock.tick(1);

Expand Down Expand Up @@ -790,10 +796,12 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);

triggerMousemoveAtElementStart(root.querySelector('rect#startrect')!);
triggerMousemoveAtElementStart(
docCanvas.querySelector('rect#startrect')!
);
// Tick the clock forward to account for the popup delay.
clock.tick(1);

Expand Down Expand Up @@ -833,10 +841,12 @@ describe('RcxContent', function () {
</text>
</g>
</svg>`,
root
docCanvas
);

triggerMousemoveAtElementStart(root.querySelector('rect#startrect')!);
triggerMousemoveAtElementStart(
docCanvas.querySelector('rect#startrect')!
);
// Tick the clock forward to account for the popup delay.
clock.tick(1);

Expand Down Expand Up @@ -873,10 +883,12 @@ describe('RcxContent', function () {
></rect>
</g>
</svg>`,
root
docCanvas
);

triggerMousemoveAtElementStart(root.querySelector('rect#startrect')!);
triggerMousemoveAtElementStart(
docCanvas.querySelector('rect#startrect')!
);
// Tick the clock forward to account for the popup delay.
clock.tick(1);

Expand Down

0 comments on commit 6809154

Please sign in to comment.