Skip to content

Commit

Permalink
View improvements (#364)
Browse files Browse the repository at this point in the history
- Refactor grind background image rendering to a css only approach
- Use @jsx annotion for view files
  Use @jsx annotation instead of const JSX definition for view files. Adapt eslint config to ignore 
  unused svg and html imports to avoid explict deactivation of the rule in each view file.
  Reason
  - Sprotty base views also use annotations
   - Annotations are more robust. E.g. mocha tests with ts-node fail with an unused local error  
   when using JSX const

- Add tests for `initializeDiagramContainer`
- Cleanup GLSPProjectionView overrides


- Also add Grid properties if grid is not visible
- Adjust variable naming in default grid css
  • Loading branch information
tortmayr authored Jun 20, 2024
1 parent 53d421e commit 7a51d29
Show file tree
Hide file tree
Showing 18 changed files with 107 additions and 142 deletions.
7 changes: 7 additions & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ module.exports = {
message:
"The sprotty-protocol default exports are customized and reexported by GLSP. Please use '@eclipse-glsp/client' instead"
}
],
'@typescript-eslint/no-unused-vars': [
'error',
{
args: 'none',
varsIgnorePattern: 'svg|html'
}
]
}
};
8 changes: 4 additions & 4 deletions examples/workflow-glsp/src/workflow-diagram-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ import {
editLabelFeature,
gridModule,
helperLineModule,
initializeDiagramContainer
initializeDiagramContainer,
overrideModelElement
} from '@eclipse-glsp/client';
import 'balloon-css/balloon.min.css';
import { Container } from 'inversify';
Expand All @@ -69,16 +70,15 @@ export const workflowDiagramModule = new FeatureModule(
configureModelElement(context, 'task:manual', TaskNode, RoundedCornerNodeView);
configureModelElement(context, 'label:heading', GLabel, GLabelView, { enable: [editLabelFeature] });
configureModelElement(context, 'comp:comp', GCompartment, GCompartmentView);
configureModelElement(context, 'comp:header', GCompartment, GCompartmentView);
configureModelElement(context, 'label:icon', GLabel, GLabelView);
configureModelElement(context, DefaultTypes.EDGE, GEdge, WorkflowEdgeView);
overrideModelElement(context, DefaultTypes.EDGE, GEdge, WorkflowEdgeView);
configureModelElement(context, 'edge:weighted', WeightedEdge, WorkflowEdgeView);
configureModelElement(context, 'icon', Icon, IconView);
configureModelElement(context, 'activityNode:merge', BranchingNode, DiamondNodeView);
configureModelElement(context, 'activityNode:decision', BranchingNode, DiamondNodeView);
configureModelElement(context, 'activityNode:fork', SynchronizationNode, RectangularNodeView);
configureModelElement(context, 'activityNode:join', SynchronizationNode, RectangularNodeView);
configureModelElement(context, DefaultTypes.GRAPH, GGraph, GLSPProjectionView);
overrideModelElement(context, DefaultTypes.GRAPH, GGraph, GLSPProjectionView);
configureModelElement(context, 'category', CategoryNode, RoundedCornerNodeView);
configureModelElement(context, 'struct', GCompartment, StructureCompartmentView);

Expand Down
4 changes: 1 addition & 3 deletions examples/workflow-glsp/src/workflow-views.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/** @jsx svg */
import {
GEdge,
Point,
Expand All @@ -30,9 +31,6 @@ import { injectable } from 'inversify';
import { VNode } from 'snabbdom';
import { Icon, isTaskNode } from './model';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const JSX = { createElement: svg };

@injectable()
export class WorkflowEdgeView extends PolylineEdgeViewWithGapsOnIntersections {
protected override renderAdditionals(edge: GEdge, segments: Point[], context: RenderingContext): VNode[] {
Expand Down
14 changes: 8 additions & 6 deletions packages/client/css/grid.css
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,14 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

:root {
--grid-color: rgba(0, 0, 0, 0.1);
}

/** Control visibility of background image through CSS class on parent */
.grid-background .sprotty-graph,
.grid-background.sprotty-graph {
background-image: var(--grid-background-image);
--grid-stroke-width: calc(1px * var(--grid-background-zoom));
--grid-stroke-width-half: calc(var(--grid-stroke-width) / 2);
--grid-color: rgba(0, 0, 0, 0.1);
background-image: linear-gradient(to right, var(--grid-color) var(--grid-stroke-width), transparent var(--grid-stroke-width)),
linear-gradient(to bottom, var(--grid-color) var(--grid-stroke-width), transparent var(--grid-stroke-width));
background-size: var(--grid-background-width) var(--grid-background-height);
background-position: calc(var(--grid-background-x) - var(--grid-stroke-width-half))
calc(var(--grid-background-y) - var(--grid-stroke-width-half));
}
56 changes: 56 additions & 0 deletions packages/client/src/default-modules.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/********************************************************************************
* Copyright (c) 2024 EclipseSource and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
import { FeatureModule } from '@eclipse-glsp/sprotty';
import { expect } from 'chai';
import { Container } from 'inversify';
import * as sinon from 'sinon';
import { defaultModule } from './base/default.module';
import { DEFAULT_MODULES, initializeDiagramContainer } from './default-modules';

describe('default-modules', () => {
// InitializeDiagramContainer internally uses `resolveContainerConfiguration` so we only test functionality
// that is not covered by the tests of `resolveContainerConfiguration`.
describe('initializeDiagramContainer', () => {
const sandbox = sinon.createSandbox();
const container = new Container();
const loadSpy = sandbox.spy(container, 'load');
container.snapshot();

beforeEach(() => {
sandbox.reset();
container.restore();
container.snapshot();
});
it('should initialize the diagram container with the default modules in addition to the given config and load them first', () => {
const extraModule = new FeatureModule(() => {});
initializeDiagramContainer(container, { add: extraModule });
expect(loadSpy.calledOnce).to.be.true;
const callArgs = loadSpy.firstCall.args;
const lastModule = callArgs.pop();
expect(callArgs).to.be.deep.equal(DEFAULT_MODULES).ordered;
expect(lastModule).to.be.equal(extraModule);
});
it('should throw an error if the base (default) module is removed via configuration', () => {
expect(() => initializeDiagramContainer(container, { remove: defaultModule })).to.throw(/Invalid module configuration/);
});
// eslint-disable-next-line max-len
it('should throw an error if the base (default) module is not the first module of the resolved configured (removed and added again)', () => {
expect(() => initializeDiagramContainer(container, { remove: defaultModule, add: defaultModule })).to.throw(
/Invalid module configuration/
);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,14 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/* eslint-disable max-len */

/** @jsx svg */
import { Bounds, GModelElement, IVNodePostprocessor, Point, isDecoration, isSizeable, setClass, svg } from '@eclipse-glsp/sprotty';
import { inject, injectable, optional } from 'inversify';
import { VNode } from 'snabbdom';
import { GGraph } from '../../model';
import { BoundsAwareModelElement } from '../../utils/gmodel-util';
import { DebugManager } from './debug-manager';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const JSX = { createElement: svg };

export const CSS_DEBUG_BOUNDS = 'debug-bounds';

@injectable()
Expand Down
4 changes: 1 addition & 3 deletions packages/client/src/features/helper-lines/view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/** @jsx svg */
import { IViewArgs, RenderingContext, ShapeView, svg } from '@eclipse-glsp/sprotty';
import { injectable } from 'inversify';
import { VNode } from 'snabbdom';
import { HelperLine, SelectionBounds } from './model';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const JSX = { createElement: svg };

@injectable()
export class HelperLineView extends ShapeView {
override render(model: HelperLine, _context: RenderingContext): VNode | undefined {
Expand Down
4 changes: 1 addition & 3 deletions packages/client/src/features/tools/change-bounds/view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/** @jsx svg */
import { IView, Point, RenderingContext, setAttr, svg } from '@eclipse-glsp/sprotty';
import { injectable } from 'inversify';
import { VNode } from 'snabbdom';
import { GResizeHandle } from '../../change-bounds/model';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const JSX = { createElement: svg };

@injectable()
export class GResizeHandleView implements IView {
render(handle: GResizeHandle, context: RenderingContext): VNode | undefined {
Expand Down
6 changes: 2 additions & 4 deletions packages/client/src/features/tools/edge-creation/view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/** @jsx svg */
import { GModelElement, IView, Point, RenderingContext, svg } from '@eclipse-glsp/sprotty';
import { injectable } from 'inversify';
import { VNode } from 'snabbdom';
import { IView, Point, RenderingContext, GModelElement, svg } from '@eclipse-glsp/sprotty';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const JSX = { createElement: svg };

/**
* This view is used for the invisible end of the feedback edge.
Expand Down
6 changes: 2 additions & 4 deletions packages/client/src/features/tools/marquee-selection/view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/** @jsx svg */
import { RectangularNodeView, RenderingContext, svg } from '@eclipse-glsp/sprotty';
import { injectable } from 'inversify';
import { VNode } from 'snabbdom';
import { RectangularNodeView, RenderingContext, svg } from '@eclipse-glsp/sprotty';
import { MarqueeNode } from './model';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const JSX = { createElement: svg };

@injectable()
export class MarqueeView extends RectangularNodeView {
override render(node: MarqueeNode, context: RenderingContext): VNode {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,14 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

/** @jsx svg */
import { IView, RenderingContext, TYPES, setClass, svg } from '@eclipse-glsp/sprotty';
import { inject, injectable, optional } from 'inversify';
import { VNode } from 'snabbdom';
import { GArgument } from '../../../utils/argument-utils';
import { Grid } from '../../grid/grid';
import { ARG_LENGTH, InsertIndicator } from './insert-indicator';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const JSX = { createElement: svg };

@injectable()
export class InsertIndicatorView implements IView {
@optional() @inject(TYPES.Grid) protected grid?: Grid;
Expand Down
6 changes: 2 additions & 4 deletions packages/client/src/views/compartments.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/** @jsx svg */
import { GCompartment, RenderingContext, ShapeView, svg } from '@eclipse-glsp/sprotty';
import { injectable } from 'inversify';
import { VNode } from 'snabbdom';
import { GCompartment, RenderingContext, ShapeView, svg } from '@eclipse-glsp/sprotty';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const JSX = { createElement: svg };

@injectable()
export class StructureCompartmentView extends ShapeView {
Expand Down
6 changes: 2 additions & 4 deletions packages/client/src/views/gedge-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/
/** @jsx svg */
import { Point, PolylineEdgeView, RenderingContext, svg } from '@eclipse-glsp/sprotty';
import { injectable } from 'inversify';
import { Classes, VNode } from 'snabbdom';
import { EdgePadding } from '../utils/argument-utils';
import { GEdge } from '../model';

// eslint-disable-next-line @typescript-eslint/no-unused-vars
const JSX = { createElement: svg };
import { EdgePadding } from '../utils/argument-utils';

@injectable()
export class GEdgeView extends PolylineEdgeView {
Expand Down
14 changes: 2 additions & 12 deletions packages/client/src/views/ggraph-view.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export class GGraphView extends SGraphView {
}

protected getGridStyle(viewport: Readonly<SGraphImpl>, context: RenderingContext): GridStyle {
if (context.targetKind === 'hidden' || !this.gridManager?.isGridVisible) {
if (context.targetKind === 'hidden' || !this.gridManager) {
return {};
}
const bounds = this.getBackgroundBounds(viewport, context, this.gridManager);
Expand All @@ -41,11 +41,7 @@ export class GGraphView extends SGraphView {
[GridProperty.GRID_BACKGROUND_Y]: bounds.y + 'px',
[GridProperty.GRID_BACKGROUND_WIDTH]: bounds.width + 'px',
[GridProperty.GRID_BACKGROUND_HEIGHT]: bounds.height + 'px',
[GridProperty.GRID_BACKGROUND_ZOOM]: viewport.zoom + '',
[GridProperty.GRID_BACKGROUND_IMAGE]: this.getBackgroundImage(viewport, context, this.gridManager),
backgroundPosition: `${bounds.x}px ${bounds.y}px`,
backgroundSize: `${bounds.width}px ${bounds.height}px`
// we do not set the background-image directly in the style object, because we want to toggle it on and off via CSS
[GridProperty.GRID_BACKGROUND_ZOOM]: viewport.zoom + ''
};
}

Expand All @@ -54,10 +50,4 @@ export class GGraphView extends SGraphView {
const size = Dimension.fromPoint(Point.multiplyScalar(gridManager.grid, viewport.zoom));
return { ...position, ...size };
}

protected getBackgroundImage(viewport: Readonly<SGraphImpl>, context: RenderingContext, gridManager: GridManager): string {
const color = getComputedStyle(document.documentElement).getPropertyValue(GridProperty.GRID_COLOR).trim().replace(/#/g, '%23');
// eslint-disable-next-line max-len
return `url('data:image/svg+xml;utf8, <svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 ${gridManager.grid.x} ${gridManager.grid.y}"><rect width="${gridManager.grid.x}" height="${gridManager.grid.y}" x="0" y="0" fill="none" stroke="${color}" stroke-width="1" /></svg>')`;
}
}
Loading

0 comments on commit 7a51d29

Please sign in to comment.