From 4bfb6d46a5254c857f2ba932d40e5b99f824b0a3 Mon Sep 17 00:00:00 2001 From: Namatuzio <55001591+Namatuzio@users.noreply.github.com> Date: Wed, 29 Nov 2023 13:36:07 -0500 Subject: [PATCH] chore: enable strict mode for workspace-minimap (#2078) * Fixed workscpace-minimap/focus_region.ts type errors Added various type checks and changes to focus_region.ts which follows proper standards for the TypeScript "strict-mode" compiler checks. * fixed type errors in workspace-minimap/minimap.ts Fixed all type error in ordinance with TypeScript's "strict-mode" * Updated workspace-minimap to utilize TS strict-mode Fully updated and fixed all type errors after enabling strict-mode. * Updated index.ts to follow strict-mode rules * fix lint errors in index.ts * Updated testing file to coordinate with strict-mode * Added requested changes Added the changes request under the PR * Added feedback changes * Removed JSDOM test code Removed the JSDOM code found in `plugins/workspace-minimap/test/minimap-test.mocha.js` as it was unnecessary due to updates to the code. * format files using prettier --- plugins/workspace-minimap/src/focus_region.ts | 21 ++-- plugins/workspace-minimap/src/minimap.ts | 98 ++++++++++++------- .../src/positioned_minimap.ts | 11 ++- plugins/workspace-minimap/test/index.ts | 12 ++- plugins/workspace-minimap/tsconfig.json | 2 +- 5 files changed, 95 insertions(+), 49 deletions(-) diff --git a/plugins/workspace-minimap/src/focus_region.ts b/plugins/workspace-minimap/src/focus_region.ts index 4211cf287d..75f2cdc5bb 100644 --- a/plugins/workspace-minimap/src/focus_region.ts +++ b/plugins/workspace-minimap/src/focus_region.ts @@ -28,9 +28,9 @@ const borderRadius = 6; */ export class FocusRegion { private onChangeWrapper: (e: Blockly.Events.Abstract) => void; - private svgGroup: SVGElement; - private rect: SVGElement; - private background: SVGElement; + private svgGroup: SVGElement | null = null; + private rect: SVGElement | null = null; + private background: SVGElement | null = null; private id: string; private initialized = false; @@ -45,6 +45,7 @@ export class FocusRegion { private minimapWorkspace: Blockly.WorkspaceSvg, ) { this.id = String(Math.random()).substring(2); + this.onChangeWrapper = this.onChange.bind(this); } /** @@ -129,7 +130,7 @@ export class FocusRegion { dispose() { if (this.onChangeWrapper) { this.primaryWorkspace.removeChangeListener(this.onChangeWrapper); - this.onChangeWrapper = null; + this.onChangeWrapper = () => null; } if (this.svgGroup) { Blockly.utils.dom.removeNode(this.svgGroup); @@ -187,9 +188,15 @@ export class FocusRegion { top += (minimapSvg.height - minimapContent.height) / 2; // Set the svg attributes. - this.rect.setAttribute('transform', `translate(${left},${top})`); - this.rect.setAttribute('width', width.toString()); - this.rect.setAttribute('height', height.toString()); + if (!this.rect) { + throw new Error( + 'The focus region must be initialized (`init`) before calling `update`', + ); + } else { + this.rect.setAttribute('transform', `translate(${left},${top})`); + this.rect.setAttribute('width', width.toString()); + this.rect.setAttribute('height', height.toString()); + } } /** diff --git a/plugins/workspace-minimap/src/minimap.ts b/plugins/workspace-minimap/src/minimap.ts index f15ce3a0e9..14e795a6b1 100644 --- a/plugins/workspace-minimap/src/minimap.ts +++ b/plugins/workspace-minimap/src/minimap.ts @@ -30,12 +30,12 @@ const blockEvents = new Set([ */ export class Minimap { protected primaryWorkspace: Blockly.WorkspaceSvg; - protected minimapWorkspace: Blockly.WorkspaceSvg; - protected focusRegion: FocusRegion; - protected onMouseMoveWrapper: Blockly.browserEvents.Data; - protected onMouseDownWrapper: Blockly.browserEvents.Data; - protected onMouseUpWrapper: Blockly.browserEvents.Data; - protected minimapWrapper: HTMLDivElement; + protected minimapWorkspace: Blockly.WorkspaceSvg | null = null; + protected focusRegion: FocusRegion | null = null; + protected onMouseMoveWrapper: Blockly.browserEvents.Data | null = null; + protected onMouseDownWrapper: Blockly.browserEvents.Data | null = null; + protected onMouseUpWrapper: Blockly.browserEvents.Data | null = null; + protected minimapWrapper: HTMLDivElement | null = null; /** * Constructor for a minimap. @@ -50,15 +50,22 @@ export class Minimap { * Initialize. */ init(): void { + const primaryInjectParentDiv = + this.primaryWorkspace.getInjectionDiv().parentNode; + + if (!primaryInjectParentDiv) { + throw new Error( + 'The workspace must be injected into the page before the minimap can be initalized', + ); + } + // Create a wrapper div for the minimap injection. this.minimapWrapper = document.createElement('div'); this.minimapWrapper.id = 'minimapWrapper' + this.primaryWorkspace.id; this.minimapWrapper.className = 'blockly-minimap'; // Make the wrapper a sibling to the primary injection div. - const primaryInjectParentDiv = - this.primaryWorkspace.getInjectionDiv().parentNode; - primaryInjectParentDiv.appendChild(this.minimapWrapper); + primaryInjectParentDiv?.appendChild(this.minimapWrapper); // Inject the minimap workspace. this.minimapWorkspace = Blockly.inject(this.minimapWrapper.id, { @@ -74,18 +81,20 @@ export class Minimap { // Remove the scale bounds of the minimap so that it can // correctly zoomToFit. zoom: { - maxScale: null, - minScale: null, + maxScale: Infinity, + minScale: 0, }, readOnly: true, theme: this.primaryWorkspace.getTheme(), renderer: this.primaryWorkspace.options.renderer, }); - this.minimapWorkspace.scrollbar.setContainerVisible(false); + this.minimapWorkspace.scrollbar?.setContainerVisible(false); this.primaryWorkspace.addChangeListener((e) => void this.mirror(e)); window.addEventListener('resize', () => { - this.minimapWorkspace.zoomToFit(); + if (this.minimapWorkspace) { + this.minimapWorkspace.zoomToFit(); + } }); // The mouseup binds to the parent container div instead of the minimap @@ -121,7 +130,9 @@ export class Minimap { if (this.isFocusEnabled()) { this.disableFocusRegion(); } - this.minimapWorkspace.dispose(); + if (this.minimapWorkspace) { + this.minimapWorkspace.dispose(); + } Blockly.utils.dom.removeNode(this.minimapWrapper); if (this.onMouseMoveWrapper) { Blockly.browserEvents.unbind(this.onMouseMoveWrapper); @@ -153,13 +164,17 @@ export class Minimap { } // Run the event in the minimap. const json = event.toJson(); - const duplicate = Blockly.Events.fromJson(json, this.minimapWorkspace); - duplicate.run(true); + if (this.minimapWorkspace) { + const duplicate = Blockly.Events.fromJson(json, this.minimapWorkspace); + duplicate.run(true); + } // Resize and center the minimap. // We need to wait for the event to finish rendering to do the zoom. Blockly.renderManagement.finishQueuedRenders().then(() => { - this.minimapWorkspace.zoomToFit(); + if (this.minimapWorkspace) { + this.minimapWorkspace.zoomToFit(); + } }); } @@ -205,13 +220,19 @@ export class Minimap { * @param event The minimap browser event. */ private primaryScroll(event: PointerEvent): void { - const [x, y] = Minimap.minimapToPrimaryCoords( - this.primaryWorkspace.getMetrics(), - this.minimapWorkspace.getMetrics(), - event.offsetX, - event.offsetY, - ); - this.primaryWorkspace.scroll(x, y); + const primaryMetrics = this.primaryWorkspace.getMetrics(); + if (this.minimapWorkspace) { + const minimapMetrics = this.minimapWorkspace.getMetrics(); + if (primaryMetrics && minimapMetrics) { + const [x, y] = Minimap.minimapToPrimaryCoords( + primaryMetrics, + minimapMetrics, + event.offsetX, + event.offsetY, + ); + this.primaryWorkspace.scroll(x, y); + } + } } /** @@ -220,13 +241,15 @@ export class Minimap { * @param event The minimap browser event. */ private onClickDown(event: PointerEvent): void { - this.onMouseMoveWrapper = Blockly.browserEvents.bind( - this.minimapWorkspace.svgGroup_, - 'mousemove', - this, - this.onMouseMove, - ); - this.primaryScroll(event); + if (this.minimapWorkspace) { + this.onMouseMoveWrapper = Blockly.browserEvents.bind( + this.minimapWorkspace.svgGroup_, + 'mousemove', + this, + this.onMouseMove, + ); + this.primaryScroll(event); + } } /** @@ -252,14 +275,18 @@ export class Minimap { * Enables the focus region; A highlight of the viewport in the minimap. */ enableFocusRegion(): void { - this.focusRegion.init(); + if (this.focusRegion) { + this.focusRegion.init(); + } } /** * Disables the focus region. */ disableFocusRegion(): void { - this.focusRegion.dispose(); + if (this.focusRegion) { + this.focusRegion.dispose(); + } } /** @@ -268,6 +295,9 @@ export class Minimap { * @returns True if the focus region is enabled. */ isFocusEnabled(): boolean { - return this.focusRegion.isEnabled(); + if (this.focusRegion) { + return this.focusRegion.isEnabled(); + } + return false; } } diff --git a/plugins/workspace-minimap/src/positioned_minimap.ts b/plugins/workspace-minimap/src/positioned_minimap.ts index 05295f74df..e17abde1b2 100644 --- a/plugins/workspace-minimap/src/positioned_minimap.ts +++ b/plugins/workspace-minimap/src/positioned_minimap.ts @@ -178,15 +178,20 @@ export class PositionedMinimap * Sets the CSS attribute for the minimap. */ private setAttributes(): void { - const injectDiv = this.minimapWorkspace.getInjectionDiv(); - const style = injectDiv.parentElement.style; + const injectDiv = this.minimapWorkspace?.getInjectionDiv(); + if (injectDiv?.parentElement === null) { + return; + } + const style = injectDiv!.parentElement.style; style.zIndex = '2'; style.position = 'absolute'; style.width = `${this.width}px`; style.height = `${this.height}px`; style.top = `${this.top}px`; style.left = `${this.left}px`; - Blockly.svgResize(this.minimapWorkspace); + if (this.minimapWorkspace) { + Blockly.svgResize(this.minimapWorkspace); + } } } diff --git a/plugins/workspace-minimap/test/index.ts b/plugins/workspace-minimap/test/index.ts index 55e9644480..a05763b5e2 100644 --- a/plugins/workspace-minimap/test/index.ts +++ b/plugins/workspace-minimap/test/index.ts @@ -12,8 +12,7 @@ import * as Blockly from 'blockly'; import {toolboxCategories, createPlayground} from '@blockly/dev-tools'; import {PositionedMinimap} from '../src/index'; -let minimap = null; -let workspace = null; +let minimap: PositionedMinimap | null = null; /** * Create a workspace. @@ -30,7 +29,7 @@ function createWorkspace( if (minimap) { minimap.dispose(); } - workspace = Blockly.inject(blocklyDiv, options); + const workspace = Blockly.inject(blocklyDiv, options); minimap = new PositionedMinimap(workspace); minimap.init(); @@ -38,7 +37,12 @@ function createWorkspace( } document.addEventListener('DOMContentLoaded', function () { - createPlayground(document.getElementById('root'), createWorkspace, { + const rootElement = document.getElementById('root'); + if (rootElement === null) { + console.error("No element with id 'root' found"); + return; + } + createPlayground(rootElement, createWorkspace, { toolbox: toolboxCategories, }); }); diff --git a/plugins/workspace-minimap/tsconfig.json b/plugins/workspace-minimap/tsconfig.json index f57c1467f3..346064aef2 100644 --- a/plugins/workspace-minimap/tsconfig.json +++ b/plugins/workspace-minimap/tsconfig.json @@ -8,7 +8,7 @@ "module": "es2015", "moduleResolution": "bundler", "target": "es6", - "strict": false, + "strict": true, // Point at the local Blockly. See #1934. Remove if we add hoisting. "paths": { "blockly/*": ["node_modules/blockly/*"]