Repository: maxGraph/maxGraph
Pull request #407 (2024-05-21 13:11:39)

fix: correctly set the default CSS class name for MaxWindow
Restore the class name used in mxGraph to increase compatibility.
Previously, the default value was empty.

Also update the CSS rules common.css to correctly use the `mxWindow` classes.
This restores the original mxGraph CSS content which should ease the migration.

Notice that this change allows to fix the stories involving `MaxWindow`:
  - HelloPort
  - JSonData
  - UserObjects
  - Windows

The Windows story has been migrated to TypeScript to ease the maintenance.
The popup are now made closable. Currently, when switching from the Windows story to another one, the popup are not destroyed and are still displayed in the other story. Making them closable provide a visual workaround to remove them manually.
Solving this problem is outside the scope of the changes proposed here. It also has an impact on all stories using `MaxWindow` or other additions of graphical elements to the graph (like `MaxPopupMenu`, for example).

## Notes

mxGraph implementation: https://github.com/jgraph/mxgraph/blob/v4.2.2/javascript/src/js/util/mxWindow.js#L311
About popup not removed when quiting the story, see also #400.


### Fixes in Stories

Story | before fix | after fix
---- | ---- | ----
UserObjects | ![maxGraph_story_UserObjects_01_before_fix](https://github.com/maxGraph/maxGraph/assets/27200110/b0e2a978-1d09-46ca-afe1-c2dd27c8f55c) | ![maxGraph_story_UserObjects_02_after_fix](https://github.com/maxGraph/maxGraph/assets/27200110/0cc4bd62-f437-48e1-a85f-f15ffa1d6c78)
Windows | ![maxgraph_story_windows_01_before_fix](https://github.com/maxGraph/maxGraph/assets/27200110/53ba63d1-7b0f-406a-a486-82c7fbd6d462) | ![maxgraph_story_windows_02_after_fix](https://github.com/maxGraph/maxGraph/assets/27200110/9abf9d6c-095e-4a74-a99f-c53d9f29b107)





Patch:
diff --git a/packages/core/css/common.css b/packages/core/css/common.css
index f7eb430ae..21f389211 100644
--- a/packages/core/css/common.css
+++ b/packages/core/css/common.css
@@ -25,7 +25,7 @@ div.mxRubberband {
 	padding: 0px;
 	margin: 0px;
 }
-div.MaxWindow {
+div.mxWindow {
 	-webkit-box-shadow: 3px 3px 12px #C0C0C0;
 	-moz-box-shadow: 3px 3px 12px #C0C0C0;
 	box-shadow: 3px 3px 12px #C0C0C0;
@@ -36,7 +36,7 @@ div.MaxWindow {
 	overflow: hidden;
 	z-index: 1;
 }
-table.MaxWindow {
+table.mxWindow {
 	border-collapse: collapse;
 	table-layout: fixed;
   	font-family: Arial;
diff --git a/packages/core/src/gui/MaxWindow.ts b/packages/core/src/gui/MaxWindow.ts
index 2cb6af317..54cec9c83 100644
--- a/packages/core/src/gui/MaxWindow.ts
+++ b/packages/core/src/gui/MaxWindow.ts
@@ -188,7 +188,7 @@ class MaxWindow extends EventSource {
     minimizable = true,
     movable = true,
     replaceNode: HTMLElement | null = null,
-    style = ''
+    style?: string
   ) {
     super();
 
@@ -235,7 +235,7 @@ class MaxWindow extends EventSource {
     y: number,
     width: number | null = null,
     height: number | null = null,
-    style = 'MaxWindow'
+    style = 'mxWindow'
   ): void {
     this.div = document.createElement('div');
     this.div.className = style;
diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts
index 362c86e94..9718ae826 100644
--- a/packages/core/src/types.ts
+++ b/packages/core/src/types.ts
@@ -197,7 +197,7 @@ export type CellStateStyle = {
   endFill?: boolean;
   /**
    * Set the fill color of the end arrow marker if {@link endFill} is `true`.
-   * If not set, use {@link startStrokeColor}.
+   * If not set, use {@link strokeColor}.
    * @since 0.10.0
    */
   endFillColor?: ColorValue;
@@ -257,11 +257,13 @@ export type CellStateStyle = {
    */
   exitY?: number;
   /**
-   * The possible values are all HTML color names or HEX codes, as well as special keywords such
-   * as `swimlane`, `inherit`, `indicated` to use the color code of a related cell or the
-   * indicator shape.
+   * The possible values are all HTML color names or HEX codes, as well as special keywords such as:
+   * - `indicated` to use the color of a related cell or the indicator shape
+   * - `inherit` to use the color of the direct parent cell
+   * - `none` for no color
+   * - `swimlane` to use the color of the parent swimlane if one exists in the parent hierarchy
    */
-  fillColor?: ColorValue;
+  fillColor?: SpecialStyleColorValue;
   /**
    * Possible range is `0-100`.
    */
@@ -290,9 +292,13 @@ export type CellStateStyle = {
    */
   foldable?: boolean;
   /**
-   * The possible values are all HTML color names or HEX codes.
+   * The possible values are all HTML color names or HEX codes, as well as special keywords such as:
+   * - `indicated` to use the color of a related cell or the indicator shape
+   * - `inherit` to use the color of the direct parent cell
+   * - `none` for no color
+   * - `swimlane` to use the color of the parent swimlane if one exists in the parent hierarchy
    */
-  fontColor?: ColorValue;
+  fontColor?: SpecialStyleColorValue;
   /**
    * The possible values are names such as `Arial; Dialog; Verdana; Times New Roman`.
    */
@@ -312,9 +318,13 @@ export type CellStateStyle = {
    */
   glass?: boolean;
   /**
-   * The possible values are all HTML color names or HEX codes.
+   * The possible values are all HTML color names or HEX codes, as well as special keywords such as:
+   * - `indicated` to use the color of a related cell or the indicator shape
+   * - `inherit` to use the color of the direct parent cell
+   * - `none` for no color
+   * - `swimlane` to use the color of the parent swimlane if one exists in the parent hierarchy
    */
-  gradientColor?: ColorValue;
+  gradientColor?: SpecialStyleColorValue;
   /**
    * Generally, and by default in maxGraph, gradient painting is done from the value of {@link fillColor} to the value of {@link gradientColor}.
    * If we take the example of 'north', this means that the {@link fillColor} color is at the bottom of paint pattern
@@ -733,11 +743,13 @@ export type CellStateStyle = {
    */
   startSize?: number;
   /**
-   * The possible values are all HTML color names or HEX codes, as well as special keywords such
-   * as `swimlane`, `inherit`, `indicated` to use the color code of a related cell or the
-   * indicator shape or `none` for no color.
+   * The possible values are all HTML color names or HEX codes, as well as special keywords such as:
+   * - `indicated` to use the color of a related cell or the indicator shape
+   * - `inherit` to use the color of the direct parent cell
+   * - `none` for no color
+   * - `swimlane` to use the color of the parent swimlane if one exists in the parent hierarchy
    */
-  strokeColor?: ColorValue;
+  strokeColor?: SpecialStyleColorValue;
   /**
    * The possible range is `0-100`.
    */
@@ -752,7 +764,7 @@ export type CellStateStyle = {
   /**
    * The fill color of the `swimlane` background.
    * The possible values are all HTML color names or HEX codes.
-   * @default no backgroune
+   * @default no background
    */
   swimlaneFillColor?: ColorValue;
   /**
@@ -841,6 +853,14 @@ export type NumericCellStateStyleKeys = NonNullable<
 >;
 
 export type ColorValue = string;
+/** Color values and special placeholders used to resolve colors (see {@link CellRenderer.resolveColor}) for style properties. */
+export type SpecialStyleColorValue =
+  | 'indicated'
+  | 'inherit'
+  | 'none'
+  | 'swimlane'
+  | (string & {});
+
 export type DirectionValue = 'north' | 'south' | 'east' | 'west';
 export type TextDirectionValue = '' | 'ltr' | 'rtl' | 'auto';
 export type AlignValue = 'left' | 'center' | 'right';
diff --git a/packages/core/src/view/style/Stylesheet.ts b/packages/core/src/view/style/Stylesheet.ts
index 01f01e877..ce52c644b 100644
--- a/packages/core/src/view/style/Stylesheet.ts
+++ b/packages/core/src/view/style/Stylesheet.ts
@@ -164,6 +164,9 @@ export class Stylesheet {
    *   - registered styles referenced in `baseStyleNames`, in the order of the array
    *   - `cellStyle` parameter
    *
+   * To fully unset a style property i.e. the property is not set even if a value is set in the default style or in the referenced styles,
+   * set the `cellStyle` property to `none`. For example. `cellStyle.fillColor = 'none'`
+   *
    * @param cellStyle An object that represents the style.
    * @param defaultStyle Default style used as reference to compute the returned style.
    */
@@ -186,15 +189,13 @@ export class Stylesheet {
     }
 
     // Merges cellStyle into style
-    const filteredCellStyle = clone(cellStyle); // Clones the cellStyle to avoid modifying the original object pass as parameter
-    for (const key of Object.keys(filteredCellStyle)) {
-      (filteredCellStyle[key] === undefined || filteredCellStyle[key] == NONE) &&
-        delete filteredCellStyle[key];
+    for (const key of Object.keys(cellStyle)) {
+      // @ts-ignore
+      if (cellStyle[key] !== undefined) {
+        // @ts-ignore
+        cellStyle[key] == NONE ? delete style[key] : (style[key] = cellStyle[key]);
+      }
     }
-    style = {
-      ...style,
-      ...filteredCellStyle,
-    };
 
     // Remove the 'baseStyleNames' that may have been copied from the cellStyle parameter to match the method signature
     'baseStyleNames' in style && delete style.baseStyleNames;
diff --git a/packages/html/stories/ColorStylePlaceHolders.stories.ts b/packages/html/stories/ColorStylePlaceHolders.stories.ts
new file mode 100644
index 000000000..a1d1d1611
--- /dev/null
+++ b/packages/html/stories/ColorStylePlaceHolders.stories.ts
@@ -0,0 +1,123 @@
+/*
+Copyright 2024-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 { constants, Graph } from '@maxgraph/core';
+import { globalTypes, globalValues } from './shared/args.js';
+import { configureImagesBasePath, createGraphContainer } from './shared/configure.js';
+
+export default {
+  title: 'Styles/ColorStylePlaceHolders',
+  argTypes: {
+    ...globalTypes,
+  },
+  args: {
+    ...globalValues,
+  },
+};
+
+const Template = ({ label, ...args }: Record<string, string>) => {
+  configureImagesBasePath();
+  const container = createGraphContainer(args);
+
+  // Creates the graph inside the given container
+  const graph = new Graph(container);
+
+  // Disables global features
+  graph.cellsSelectable = false;
+  graph.cellsLocked = true;
+
+  // Sets global styles
+  const defaultVertexStyle = graph.getStylesheet().getDefaultVertexStyle();
+  defaultVertexStyle.foldable = false;
+
+  graph.getStylesheet().putCellStyle('swimlane', {
+    shape: constants.SHAPE.SWIMLANE,
+    startSize: 30,
+    fillColor: '#ffffff',
+    strokeColor: 'red',
+    swimlaneLine: true,
+    swimlaneFillColor: '#ffffff',
+  });
+
+  // Gets the default parent for inserting new cells. This
+  // is normally the first child of the root (ie. layer 0).
+  const parent = graph.getDefaultParent();
+
+  // Adds cells to the model in a single step
+  graph.batchUpdate(() => {
+    const swimlane = graph.insertVertex(parent, null, 'swimlane', 0, 0, 640, 400, {
+      baseStyleNames: ['swimlane'],
+    });
+
+    const parentInSwimlane = graph.insertVertex({
+      parent: swimlane,
+      value: 'Parent in swimlane',
+      position: [20, 80],
+      size: [400, 230],
+      style: { fillColor: '#e8dbdb', strokeColor: 'black' },
+    });
+
+    graph.insertVertex({
+      parent: parentInSwimlane,
+      value: 'child with none stroke color',
+      position: [10, 10],
+      size: [180, 30],
+      style: { strokeColor: 'none' },
+    });
+
+    graph.insertVertex({
+      parent: parentInSwimlane,
+      value: 'child with swimlane stroke color',
+      position: [205, 60],
+      size: [180, 30],
+      style: { strokeColor: 'swimlane' },
+    });
+    graph.insertVertex({
+      parent: parentInSwimlane,
+      value: 'child with swimlane fill color',
+      position: [10, 60],
+      size: [180, 30],
+      style: { fillColor: 'swimlane' },
+    });
+
+    graph.insertVertex({
+      parent: parentInSwimlane,
+      value: 'child with inherit stroke color',
+      position: [10, 160],
+      size: [180, 30],
+      style: { strokeColor: 'inherit' },
+    });
+    graph.insertVertex({
+      parent: parentInSwimlane,
+      value: 'child with inherit fill color',
+      position: [205, 160],
+      size: [180, 30],
+      style: { fillColor: 'inherit' },
+    });
+
+    graph.insertVertex({
+      // parent: swimlane,
+      value: 'vertex with none fill color',
+      position: [10, 410],
+      size: [180, 30],
+      style: { fillColor: 'none' },
+    });
+  });
+
+  return container;
+};
+
+export const Default = Template.bind({});
diff --git a/packages/html/stories/HelloPort.stories.ts b/packages/html/stories/HelloPort.stories.ts
index 788e3b1b7..95aab27c0 100644
--- a/packages/html/stories/HelloPort.stories.ts
+++ b/packages/html/stories/HelloPort.stories.ts
@@ -32,8 +32,7 @@ import {
   rubberBandValues,
 } from './shared/args.js';
 import { configureImagesBasePath, createGraphContainer } from './shared/configure.js';
-// style required by RubberBand and popup
-import '@maxgraph/core/css/common.css';
+import '@maxgraph/core/css/common.css'; // style required by RubberBand and popup
 
 export default {
   title: 'Connections/HelloPort',
diff --git a/packages/html/stories/JsonData.stories.ts b/packages/html/stories/JsonData.stories.ts
index 0c005f494..6aa2d0d93 100644
--- a/packages/html/stories/JsonData.stories.ts
+++ b/packages/html/stories/JsonData.stories.ts
@@ -34,8 +34,7 @@ import {
   rubberBandValues,
 } from './shared/args.js';
 import { configureImagesBasePath, createGraphContainer } from './shared/configure.js';
-// style required by RubberBand
-import '@maxgraph/core/css/common.css';
+import '@maxgraph/core/css/common.css'; // style required by RubberBand and popup
 
 export default {
   title: 'Xml_Json/JsonData',
diff --git a/packages/html/stories/UserObject.stories.ts b/packages/html/stories/UserObject.stories.ts
index 10105d668..6b0e9c750 100644
--- a/packages/html/stories/UserObject.stories.ts
+++ b/packages/html/stories/UserObject.stories.ts
@@ -39,6 +39,7 @@ import {
   rubberBandValues,
 } from './shared/args.js';
 import { configureImagesBasePath, createGraphContainer } from './shared/configure.js';
+import '@maxgraph/core/css/common.css'; // style required by popup
 
 export default {
   title: 'Xml_Json/UserObject',
diff --git a/packages/html/stories/Window.stories.js b/packages/html/stories/Window.stories.ts
similarity index 91%
rename from packages/html/stories/Window.stories.js
rename to packages/html/stories/Window.stories.ts
index 7dba159c7..822277d86 100644
--- a/packages/html/stories/Window.stories.js
+++ b/packages/html/stories/Window.stories.ts
@@ -33,8 +33,7 @@ import {
   rubberBandValues,
 } from './shared/args.js';
 import { configureImagesBasePath, createGraphContainer } from './shared/configure.js';
-// style required by RubberBand
-import '@maxgraph/core/css/common.css';
+import '@maxgraph/core/css/common.css'; // style required by RubberBand and MaxWindow/MaxLog
 
 export default {
   title: 'Windows/Windows',
@@ -50,7 +49,7 @@ export default {
   },
 };
 
-const Template = ({ label, ...args }) => {
+const Template = ({ label, ...args }: Record<string, string>) => {
   configureImagesBasePath();
   const container = createGraphContainer(args);
 
@@ -88,12 +87,13 @@ const Template = ({ label, ...args }) => {
   graph.batchUpdate(() => {
     const v1 = graph.insertVertex(parent, null, 'Hello,', 20, 20, 80, 30);
     const v2 = graph.insertVertex(parent, null, 'World!', 200, 150, 80, 30);
-    const e1 = graph.insertEdge(parent, null, '', v1, v2);
+    graph.insertEdge(parent, null, '', v1, v2);
   });
 
   wnd.setMaximizable(true);
   wnd.setResizable(true);
   wnd.setVisible(true);
+  wnd.setClosable(true);
 
   const lorem =
     'Lorem ipsum dolor sit amet, consectetur adipisici elit, sed eiusmod tempor incidunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquid ex ea commodi consequat. Quis aute iure reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint obcaecat cupiditat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum. ';
@@ -114,8 +114,9 @@ const Template = ({ label, ...args }) => {
   wnd.setScrollable(true);
   wnd.setResizable(true);
   wnd.setVisible(true);
+  wnd.setClosable(true);
 
-  content = content.cloneNode(true);
+  content = content.cloneNode(true) as HTMLDivElement;
   content.style.width = '400px';
 
   wnd = new MaxWindow(
@@ -132,6 +133,7 @@ const Template = ({ label, ...args }) => {
   wnd.setScrollable(true);
   wnd.setResizable(true);
   wnd.setVisible(true);
+  wnd.setClosable(true);
 
   MaxLog.show();
 


Test patch:
diff --git a/packages/core/__tests__/view/style/StyleSheet.test.ts b/packages/core/__tests__/view/style/StyleSheet.test.ts
index e1604219d..cca66bf51 100644
--- a/packages/core/__tests__/view/style/StyleSheet.test.ts
+++ b/packages/core/__tests__/view/style/StyleSheet.test.ts
@@ -199,25 +199,27 @@ describe('getCellStyle', () => {
   });
 
   // This was the mxGraph behaviour, see https://github.com/jgraph/mxgraph/blob/v4.2.2/javascript/src/js/view/mxStylesheet.js#L236-L239
-  test('Setting properties with "none" value fallback to property value of the default and "base" styles', () => {
+  test('Setting properties with "none" value remove the property value', () => {
     const stylesheet = new Stylesheet();
     stylesheet.putCellStyle('style-1', {
       fillColor: 'orange',
+      rounded: true,
     });
 
     const cellStyle = stylesheet.getCellStyle(
       {
         baseStyleNames: ['style-1'],
         fillColor: NONE,
+        gradientColor: NONE, // not set in default and baseStyles
         shape: 'cloud',
         strokeColor: NONE,
       },
-      { strokeColor: 'pink' }
+      { opacity: 50, strokeColor: 'pink' }
     );
     expect(cellStyle).toStrictEqual({
-      fillColor: 'orange',
+      opacity: 50, // from default
+      rounded: true, // from style-1
       shape: 'cloud',
-      strokeColor: 'pink',
     });
   });