From b7a322b36f03c0ea4efa3ea8cb9e10808f870302 Mon Sep 17 00:00:00 2001 From: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com> Date: Fri, 16 Dec 2022 16:51:51 +0100 Subject: [PATCH] fix: CellOverlay constructor use all parameters to set properties (#159) The `align` and `verticalAlign` values passed to the constructor weren't set, so the overlay position was always set to the default one. The `align` and `verticalAlign` properties are now using the AlignValue and VAlignValue types respectively instead of defining inline types. This improves the consistent in the whole code. To demonstrate the fix, the Overlays story now set 'align' and 'verticalAlign' randomly. Also introduce `jest` to test the fix and the whole implementation of the changed method. Types check support is provided by `ts-jest`. As maxGraph uses a lot of browser objects, also setup `jest-jsdom-environment`. --- .github/workflows/build.yml | 3 ++ .../__tests__/view/cell/CellOverlay.test.ts | 40 +++++++++++++++++++ packages/core/jest.config.cjs | 6 +++ packages/core/package.json | 8 +++- packages/core/src/view/cell/CellOverlay.ts | 28 ++++++++----- packages/html/stories/Overlays.stories.js | 17 ++++++-- 6 files changed, 85 insertions(+), 17 deletions(-) create mode 100644 packages/core/__tests__/view/cell/CellOverlay.test.ts create mode 100644 packages/core/jest.config.cjs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 751a38c3b..07816276e 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -49,6 +49,9 @@ jobs: - name: Generate @maxgraph/core types working-directory: packages/core run: npm run generate-types + - name: Test @maxgraph/core + working-directory: packages/core + run: npm test - name: Test TypeScript support working-directory: packages/ts-support run: npm test diff --git a/packages/core/__tests__/view/cell/CellOverlay.test.ts b/packages/core/__tests__/view/cell/CellOverlay.test.ts new file mode 100644 index 000000000..2923d878d --- /dev/null +++ b/packages/core/__tests__/view/cell/CellOverlay.test.ts @@ -0,0 +1,40 @@ +/* +Copyright 2022-present The maxGraph project Contributors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { describe, expect, test } from '@jest/globals'; +import { CellOverlay, Point } from '../../../src'; + +test('Constructor set all parameters', () => { + const cellOverlay = new CellOverlay( + // @ts-ignore + null, + 'my tooltip', + 'left', + 'middle', + new Point(10, 20), + 'custom' + ); + + expect(cellOverlay).toEqual( + expect.objectContaining({ + align: 'left', + cursor: 'custom', + offset: new Point(10, 20), + tooltip: 'my tooltip', + verticalAlign: 'middle', + }) + ); +}); diff --git a/packages/core/jest.config.cjs b/packages/core/jest.config.cjs new file mode 100644 index 000000000..95a72e799 --- /dev/null +++ b/packages/core/jest.config.cjs @@ -0,0 +1,6 @@ +/** @type {import('ts-jest').JestConfigWithTsJest} */ +module.exports = { + // preset: 'ts-jest', + preset: 'ts-jest/presets/default-esm', + testEnvironment: 'jsdom', // need to access to the browser objects +}; diff --git a/packages/core/package.json b/packages/core/package.json index ab7cf4134..9daed5cf7 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -22,11 +22,15 @@ "build": "cross-env NODE_ENV=production webpack --mode=production", "generate-types": "tsc --emitDeclarationOnly", "generate-esm": "tsc --emitDeclarationOnly false --declaration false --declarationDir null", - "prepack": "run-s generate-types generate-esm build" + "prepack": "run-s generate-types generate-esm build", + "test": "jest" }, "devDependencies": { "circular-dependency-plugin": "^5.2.2", - "npm-run-all": "~4.1.5" + "jest": "^29.3.1", + "jest-environment-jsdom": "^29.3.1", + "npm-run-all": "~4.1.5", + "ts-jest": "^29.0.3" }, "sideEffects": true } diff --git a/packages/core/src/view/cell/CellOverlay.ts b/packages/core/src/view/cell/CellOverlay.ts index b14d9a6c3..fd09bb74e 100644 --- a/packages/core/src/view/cell/CellOverlay.ts +++ b/packages/core/src/view/cell/CellOverlay.ts @@ -22,6 +22,7 @@ import EventSource from '../event/EventSource'; import ImageBox from '../image/ImageBox'; import CellState from './CellState'; import ObjectIdentity from '../../util/ObjectIdentity'; +import { AlignValue, VAlignValue } from '../../types'; /** * Extends {@link EventSource} to implement a graph overlay, represented by an icon @@ -77,8 +78,8 @@ class CellOverlay extends EventSource implements ObjectIdentity { constructor( image: ImageBox, tooltip: string | null = null, - align = 'right', - verticalAlign = 'bottom', + align: AlignValue = 'right', + verticalAlign: VAlignValue = 'bottom', offset: Point = new Point(), cursor = 'help' ) { @@ -86,6 +87,8 @@ class CellOverlay extends EventSource implements ObjectIdentity { this.image = image; this.tooltip = tooltip; + this.align = align; + this.verticalAlign = verticalAlign; this.offset = offset; this.cursor = cursor; } @@ -101,18 +104,20 @@ class CellOverlay extends EventSource implements ObjectIdentity { tooltip?: string | null; /** - * Holds the horizontal alignment for the overlay. Default is - * {@link Constants#ALIGN_RIGHT}. For edges, the overlay always appears in the - * center of the edge. + * Holds the horizontal alignment for the overlay. + * + * For edges, the overlay always appears in the center of the edge. + * @default 'right' */ - align: 'left' | 'center' | 'right' = 'right'; + align: AlignValue = 'right'; /** - * Holds the vertical alignment for the overlay. Default is - * {@link Constants#ALIGN_BOTTOM}. For edges, the overlay always appears in the - * center of the edge. + * Holds the vertical alignment for the overlay. + * + * For edges, the overlay always appears in the center of the edge. + * @default 'bottom' */ - verticalAlign: 'top' | 'middle' | 'bottom' = 'bottom'; + verticalAlign: VAlignValue = 'bottom'; /** * Holds the offset as an {@link Point}. The offset will be scaled according to the @@ -121,7 +126,8 @@ class CellOverlay extends EventSource implements ObjectIdentity { offset = new Point(); /** - * Holds the cursor for the overlay. Default is 'help'. + * Holds the cursor for the overlay. + * @default 'help'. */ cursor = 'help'; diff --git a/packages/html/stories/Overlays.stories.js b/packages/html/stories/Overlays.stories.js index db7ca7f89..df662b38e 100644 --- a/packages/html/stories/Overlays.stories.js +++ b/packages/html/stories/Overlays.stories.js @@ -54,6 +54,14 @@ const Template = ({ label, ...args }) => { // Enables tooltips for the overlays graph.setTooltips(true); + function pickAlignValueRandomly() { + return ['left', 'center', 'right'][Math.floor(Math.random() * 3)]; + } + + function pickVerticalAlignValueRandomly() { + return ['top', 'bottom'][Math.floor(Math.random() * 2)]; + } + // Installs a handler for click events in the graph // that toggles the overlay for the respective cell graph.addListener(InternalEvent.CLICK, (sender, evt) => { @@ -61,12 +69,13 @@ const Template = ({ label, ...args }) => { if (cell != null) { const overlays = graph.getCellOverlays(cell); - - if (overlays.length == 0) { + if (overlays.length === 0) { // Creates a new overlay with an image and a tooltip const overlay = new CellOverlay( new ImageBox('/images/check.png', 16, 16), - 'Overlay tooltip' + 'Overlay tooltip', + pickAlignValueRandomly(), + pickVerticalAlignValueRandomly() ); // Installs a handler for clicks on the overlay @@ -104,7 +113,7 @@ const Template = ({ label, ...args }) => { }); const v2 = graph.insertVertex({ parent, - value: 'Doubleclick', + value: 'Double Click', position: [200, 150], size: [100, 40], });