Skip to content

Commit

Permalink
chore: enable strict mode for workspace-minimap (google#2078)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@Namatuzio @shivalipatel6
Namatuzio authored and shivalipatel6 committed Dec 4, 2023
1 parent 9c10d99 commit 5fd98d0
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 49 deletions.
21 changes: 14 additions & 7 deletions plugins/workspace-minimap/src/focus_region.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -45,6 +45,7 @@ export class FocusRegion {
private minimapWorkspace: Blockly.WorkspaceSvg,
) {
this.id = String(Math.random()).substring(2);
this.onChangeWrapper = this.onChange.bind(this);
}

/**
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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());
}
}

/**
Expand Down
98 changes: 64 additions & 34 deletions plugins/workspace-minimap/src/minimap.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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, {
Expand All @@ -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
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
}
});
}

Expand Down Expand Up @@ -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);
}
}
}

/**
Expand All @@ -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);
}
}

/**
Expand All @@ -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();
}
}

/**
Expand All @@ -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;
}
}
11 changes: 8 additions & 3 deletions plugins/workspace-minimap/src/positioned_minimap.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}

Expand Down
12 changes: 8 additions & 4 deletions plugins/workspace-minimap/test/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -30,15 +29,20 @@ function createWorkspace(
if (minimap) {
minimap.dispose();
}
workspace = Blockly.inject(blocklyDiv, options);
const workspace = Blockly.inject(blocklyDiv, options);
minimap = new PositionedMinimap(workspace);
minimap.init();

return workspace;
}

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,
});
});
2 changes: 1 addition & 1 deletion plugins/workspace-minimap/tsconfig.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/*"]
Expand Down

0 comments on commit 5fd98d0

Please sign in to comment.