Skip to content

Fixes the 'Anchors' example. #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
junsikshim merged 3 commits into from
Jan 27, 2022
Merged

Fixes the 'Anchors' example. #77

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b2bb10e
Fixed the 'Anchors' example.
junsikshim Jan 22, 2022
e750c7e
Removed .vscode from repository.
junsikshim Jan 22, 2022
93e69cc
Adds .vscode to .gitignore.
junsikshim Jan 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ javascript/dist
.jshintrc
javascript/examples/**/dist
.idea/
.vscode/
*.iml
dist
package-lock.json
Expand Down
1 change: 1 addition & 0 deletions packages/core/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare module '*.gif';
2 changes: 1 addition & 1 deletion packages/core/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export type CellStateStyles = {
defaultVertex?: CellStateStyles;
deletable?: boolean;
direction?: DirectionValue;
edge?: string;
edgeStyle?: string;

@tbouffard tbouffard Jan 25, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ This restores the original mxGraph style value. This will help people migrating from mxGraph.
Original mxGraph code: https://github.com/jgraph/mxgraph/blob/v4.2.2/javascript/src/js/util/mxConstants.js#L1796

editable?: boolean;
elbow?: string;
endArrow?: ArrowType;
Expand Down
55 changes: 22 additions & 33 deletions packages/core/src/view/GraphView.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,8 @@ import RectangleShape from './geometry/node/RectangleShape';
import { ALIGN } from '../util/Constants';
import Client from '../Client';
import InternalEvent from './event/InternalEvent';
import {
convertPoint,
getCurrentStyle,
getOffset,
} from '../util/styleUtils';
import {
getRotatedPoint,
ptSegDistSq,
relativeCcw,
toRadians,
} from '../util/mathUtils';
import { convertPoint, getCurrentStyle, getOffset } from '../util/styleUtils';
import { getRotatedPoint, ptSegDistSq, relativeCcw, toRadians } from '../util/mathUtils';
import MaxLog from '../gui/MaxLog';
import Translations from '../util/Translations';
import CellState from './cell/CellState';
Expand All @@ -49,7 +40,6 @@ import { MouseEventListener } from '../types';
import ObjectCodec from '../serialization/ObjectCodec';
import CodecRegistry from '../serialization/CodecRegistry';


/**
* @class GraphView
* @extends {EventSource}
Expand Down Expand Up @@ -236,9 +226,7 @@ export class GraphView extends EventSource {
this.viewStateChanged();
}
}
this.fireEvent(
new EventObject(InternalEvent.SCALE, { scale: value, previousScale })
);
this.fireEvent(new EventObject(InternalEvent.SCALE, { scale: value, previousScale }));
}

/**
Expand Down Expand Up @@ -415,10 +403,10 @@ export class GraphView extends EventSource {
}

this.fireEvent(
new EventObject(InternalEvent.SCALE_AND_TRANSLATE, {
scale,
previousScale,
translate: this.translate,
new EventObject(InternalEvent.SCALE_AND_TRANSLATE, {
scale,
previousScale,
translate: this.translate,
previousTranslate: previousTranslate,
})
);
Expand Down Expand Up @@ -1320,15 +1308,17 @@ export class GraphView extends EventSource {
let edgeStyle = this.isLoopStyleEnabled(edge, points, source, target)
? edge.style.loop ?? this.graph.defaultLoopStyle
: !edge.style.noEdgeStyle ?? false
? edge.style.edge
? edge.style.edgeStyle
: null;

// Converts string values to objects
if (typeof edgeStyle === 'string') {
let tmp = StyleRegistry.getValue(edgeStyle);

if (!tmp && this.isAllowEval()) {
tmp = eval(edgeStyle);
}

edgeStyle = tmp;
}

Expand Down Expand Up @@ -2267,32 +2257,32 @@ export class GraphView extends EventSource {
*/
createHtml() {
var container = this.graph.container;

if (container != null) {
this.canvas = this.createHtmlPane('100%', '100%');
this.canvas.style.overflow = 'hidden';

// Uses minimal size for inner DIVs on Canvas. This is required
// for correct event processing in IE. If we have an overlapping
// DIV then the events on the cells are only fired for labels.
this.backgroundPane = this.createHtmlPane('1px', '1px');
this.drawPane = this.createHtmlPane('1px', '1px');
this.overlayPane = this.createHtmlPane('1px', '1px');
this.decoratorPane = this.createHtmlPane('1px', '1px');

this.canvas.appendChild(this.backgroundPane);
this.canvas.appendChild(this.drawPane);
this.canvas.appendChild(this.overlayPane);
this.canvas.appendChild(this.decoratorPane);

container.appendChild(this.canvas);
this.updateContainerStyle(container);
this.updateContainerStyle(container);
}
};
}

/**
* Function: updateHtmlCanvasSize
*
*
* Updates the size of the HTML canvas.
*/
updateHtmlCanvasSize(width: number, height: number) {
Expand All @@ -2312,29 +2302,28 @@ export class GraphView extends EventSource {
this.canvas.style.height = '100%';
}
}
};
}

/**
* Function: createHtmlPane
*
*
* Creates and returns a drawing pane in HTML (DIV).
*/
createHtmlPane(width: string, height: string) {
var pane = document.createElement('DIV');

if (width != null && height != null) {
pane.style.position = 'absolute';
pane.style.left = '0px';
pane.style.top = '0px';

pane.style.width = width;
pane.style.height = height;

} else {
pane.style.position = 'relative';
}
return pane;
};
}

/**
* Updates the style of the container after installing the SVG DOM elements.
Expand All @@ -2357,8 +2346,8 @@ export class GraphView extends EventSource {
* Destroys the view and all its resources.
*/
destroy() {
let root: SVGElement | HTMLElement | null=null;
let root: SVGElement | HTMLElement | null = null;

if (this.canvas && this.canvas instanceof SVGElement) {
root = this.canvas.ownerSVGElement as SVGElement;
}
Expand Down
11 changes: 4 additions & 7 deletions packages/core/src/view/handler/ConnectionHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,7 +610,6 @@ class ConnectionHandler extends EventSource implements GraphPlugin {
* Starts a new connection for the given state and coordinates.
*/
start(state: CellState, x: number, y: number, edgeState: CellState) {
console.log("ConnectionHandler start");
this.previous = state;
this.first = new Point(x, y);
this.edgeState = edgeState ?? this.createEdgeState();
Expand Down Expand Up @@ -1191,9 +1190,8 @@ class ConnectionHandler extends EventSource implements GraphPlugin {
// Uses edge state to compute the terminal points
if (this.edgeState) {
this.updateEdgeState(current, constraint);
current = this.edgeState.absolutePoints[
this.edgeState.absolutePoints.length - 1
];
current =
this.edgeState.absolutePoints[this.edgeState.absolutePoints.length - 1];
pt2 = this.edgeState.absolutePoints[0];
} else {
if (this.currentState) {
Expand Down Expand Up @@ -1225,9 +1223,8 @@ class ConnectionHandler extends EventSource implements GraphPlugin {
let tmp = pt2;

if (this.edgeState && this.edgeState.absolutePoints.length >= 2) {
const tmp2 = this.edgeState.absolutePoints[
this.edgeState.absolutePoints.length - 2
];
const tmp2 =
this.edgeState.absolutePoints[this.edgeState.absolutePoints.length - 2];

if (tmp2) {
tmp = tmp2;
Expand Down
52 changes: 27 additions & 25 deletions packages/core/src/view/handler/ConstraintHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ import ConnectionConstraint from '../other/ConnectionConstraint';
import Point from '../geometry/Point';
import Cell from '../cell/Cell';

import pointImg from '../../../images/point.gif';

/**
* Handles constraints on connection targets. This class is in charge of
* showing fixed points when the mouse is over a vertex and handles constraints
Expand All @@ -38,7 +40,7 @@ class ConstraintHandler {
* {@link Image} to be used as the image for fixed connection points.
*/
// pointImage: mxImage;
pointImage = new Image(`${Client.imageBasePath}/point.gif`, 5, 5);
pointImage = new Image(pointImg, 5, 5);

@tbouffard tbouffard Jan 25, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ this prevents to use an alternate image
Historically, the images were not packaged withing the mxGraph bundle. When using the editor, you could provide your own image set and mxClient.imageBasePath was used to configure the path to the custom images. The same remark applies to the change in ConnectionHandler.
My comment is mainly about informing all of us that we drop this support for sure. I don't know if it was broken in maxGraph before this change.

@junsikshim junsikshim Jan 27, 2022

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd better to provide a way to overwrite those images by adding some parameters to Graph.
Maybe we could change the signature of Graph constructor a bit like (to an object),

constructor({
  container: ...,
  model: ...,
  plugins: ...,
  stylesheet: ...,
  resources: {
    ConnectionPointImageUrl: ...
  }
})

Then, the users can provide their own images while the default images are also available.

Let me know what you think!

@tbouffard tbouffard Jan 27, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this will be a better way. This can apply to other images referenced within the code.
I suggest we manage it in a dedicated Discussion/Pull Request as it goes beyond the scope of the fix we want to do here.

@mcyph mcyph Jan 27, 2022 •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we wanted to be (reasonably) back-compatible, we could set Client.imageBasePath to be null by default and use ${Client.imageBasePath}/point.gif if specified by the user or pointImg if not i.e.

pointImage = new Image(Client.imageBasePath != null ? `${Client.imageBasePath}/point.gif` : pointImg, 5, 5);

I feel more generally, it would be nice to allow setting specific user subclasses of handlers to a graph so that you could have different settings for different graphs on the same page. While subclasses might be a verbose solution, it at least is fairly consistent and haven't yet been able to think of a better solution.

@junsikshim junsikshim Jan 27, 2022

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is best to stay away from global configuration(the previous way) since it is separated from the actual Graph instantiation. When we want to create multiple different graphs on the same page, there has to be separate <script> tags containing Client.imageBasePath = ... which also could result in race conditions and so on.

I agree with @tbouffard that this should be an another topic. I'll merge this PR first, and let's discuss on this somewhere else.


/**
* Reference to the enclosing {@link mxGraph}.
Expand Down Expand Up @@ -81,8 +83,8 @@ class ConstraintHandler {
// Adds a graph model listener to update the current focus on changes
this.resetHandler = () => {
if (
this.currentFocus != null &&
this.graph.view.getState(this.currentFocus.cell) == null
this.currentFocus &&
!this.graph.view.getState(this.currentFocus.cell)
) {
this.reset();
} else {
Expand Down Expand Up @@ -211,15 +213,15 @@ class ConstraintHandler {

// Gets cell under actual point if different from event location
if (
cell == null &&
point != null &&
!cell &&
point &&
(me.getGraphX() !== point.x || me.getGraphY() !== point.y)
) {
cell = this.graph.getCellAt(point.x, point.y);
}

// Uses connectable parent vertex if one exists
if (cell != null && !cell.isConnectable()) {
if (cell && !cell.isConnectable()) {
const parent = cell.getParent();

if (parent && parent.isVertex() && parent.isConnectable()) {
Expand All @@ -246,7 +248,7 @@ class ConstraintHandler {
) {
if (this.isEnabled() && !this.isEventIgnored(me)) {
// Lazy installation of mouseleave handler
if (this.mouseleaveHandler == null && this.graph.container != null) {
if (!this.mouseleaveHandler && this.graph.container ) {
this.mouseleaveHandler = () => {
this.reset();
};
Expand All @@ -255,8 +257,8 @@ class ConstraintHandler {
}

const tol = this.getTolerance(me);
const x = point != null ? point.x : me.getGraphX();
const y = point != null ? point.y : me.getGraphY();
const x = point ? point.x : me.getGraphX();
const y = point ? point.y : me.getGraphY();
const grid = new Rectangle(x - tol, y - tol, 2 * tol, 2 * tol);
const mouse = new Rectangle(
me.getGraphX() - tol,
Expand All @@ -270,8 +272,8 @@ class ConstraintHandler {
// Keeps focus icons visible while over vertex bounds and no other cell under mouse or shift is pressed
if (
!this.isKeepFocusEvent(me) &&
(this.currentFocusArea == null ||
this.currentFocus == null ||
(!this.currentFocusArea ||
!this.currentFocus ||
state ||
!this.currentFocus.cell.isVertex() ||
!intersects(this.currentFocusArea, mouse)) &&
Expand All @@ -289,9 +291,9 @@ class ConstraintHandler {
let tmp;

if (
this.focusIcons != null &&
this.constraints != null &&
(state == null || this.currentFocus === state)
this.focusIcons.length > 0 &&
this.constraints &&
(!state || this.currentFocus === state)
) {
const cx = mouse.getCenterX();
const cy = mouse.getCenterY();
Expand All @@ -303,9 +305,9 @@ class ConstraintHandler {

if (
(this.intersects(this.focusIcons[i], mouse, source, existingEdge) ||
(point != null &&
(point &&
this.intersects(this.focusIcons[i], grid, source, existingEdge))) &&
(minDistSq == null || tmp < minDistSq)
(minDistSq === null || tmp < minDistSq)
) {
this.currentConstraint = this.constraints[i];
this.currentPoint = this.focusPoints[i];
Expand All @@ -316,7 +318,7 @@ class ConstraintHandler {
tmp.width -= 1;
tmp.height -= 1;

if (this.focusHighlight == null) {
if (!this.focusHighlight) {
const hl = this.createHighlightShape();
hl.dialect = DIALECT.SVG;
hl.pointerEvents = false;
Expand All @@ -325,7 +327,7 @@ class ConstraintHandler {
this.focusHighlight = hl;

const getState = () => {
return this.currentFocus != null ? this.currentFocus : state;
return this.currentFocus ? this.currentFocus : state;
};

InternalEvent.redirectMouseEvents(hl.node, this.graph, getState);
Expand All @@ -337,7 +339,7 @@ class ConstraintHandler {
}
}

if (this.currentConstraint == null) {
if (!this.currentConstraint) {
this.destroyFocusHighlight();
}
} else {
Expand All @@ -354,9 +356,9 @@ class ConstraintHandler {
*/
redraw() {
if (
this.currentFocus != null &&
this.constraints != null &&
this.focusIcons != null
this.currentFocus &&
this.constraints &&
this.focusIcons.length > 0
) {
const state = this.graph.view.getState(this.currentFocus.cell) as CellState;
this.currentFocus = state;
Expand Down Expand Up @@ -423,12 +425,12 @@ class ConstraintHandler {
icon.init(this.graph.getView().getDecoratorPane());

// Move the icon behind all other overlays
if (icon.node.previousSibling != null) {
if (icon.node.previousSibling) {
icon.node.parentNode?.insertBefore(icon.node, icon.node.parentNode.firstChild);
}

const getState = () => {
return this.currentFocus != null ? this.currentFocus : state;
return this.currentFocus ? this.currentFocus : state;
};

icon.redraw();
Expand Down Expand Up @@ -480,7 +482,7 @@ class ConstraintHandler {
this.graph.view.removeListener(this.resetHandler);
this.graph.removeListener(this.resetHandler);

if (this.mouseleaveHandler != null && this.graph.container != null) {
if (this.mouseleaveHandler && this.graph.container) {
InternalEvent.removeListener(
this.graph.container,
'mouseleave',
Expand Down
Loading