Skip to content

feat: added optional instance setting to allow consumers to choose si… #80

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

Open
wants to merge 1 commit into
base: release/1.0.0
Choose a base branch
from
Open

feat: added optional instance setting to allow consumers to choose si… #80

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 13 additions & 2 deletions src/views/orb-view.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ export interface IOrbViewSettings<N extends INodeBase, E extends IEdgeBase> {
export type IOrbViewSettingsInit<N extends INodeBase, E extends IEdgeBase> = Omit<
Partial<IOrbViewSettings<N, E>>,
'render'
> & { render?: Partial<IRendererSettingsInit> };
> & { render?: Partial<IRendererSettingsInit> } & {
instances: {
simulator?: ISimulator | (() => ISimulator);
};
Comment on lines +47 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> & { render?: Partial<IRendererSettingsInit> } & {
instances: {
simulator?: ISimulator | (() => ISimulator);
};
> & {
render?: Partial<IRendererSettingsInit>,
instances?: {
simulator?: ISimulator | (() => ISimulator);
};

I would say that you don't need to join them with & - just append to the render interface. Btw instances should be optional, if not users would need to always initiate an empty object even if they don't want to override the simulator instance.

};

export class OrbView<N extends INodeBase, E extends IEdgeBase> implements IOrbView<N, E, IOrbViewSettings<N, E>> {
private _container: HTMLElement;
Expand Down Expand Up @@ -150,7 +154,7 @@ export class OrbView<N extends INodeBase, E extends IEdgeBase> implements IOrbVi
.on('contextmenu', this.mouseRightClicked)
.on('dblclick.zoom', this.mouseDoubleClicked);

this._simulator = SimulatorFactory.getSimulator();
this._simulator = this._createSimulator(settings?.instances?.simulator);
this._simulator.on(SimulatorEventType.SIMULATION_START, () => {
this._isSimulating = true;
this._simulationStartedAt = Date.now();
Expand Down Expand Up @@ -569,6 +573,12 @@ export class OrbView<N extends INodeBase, E extends IEdgeBase> implements IOrbVi
this._simulator.simulate();
}


private _createSimulator(simulator: ISimulator | (() => ISimulator) | undefined): ISimulator {
if(typeof simulator === "function") return simulator();
return simulator || SimulatorFactory.getSimulator();
}
Comment on lines +577 to +580
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I think lint is not working for you because this should be indented + if should be wrapped in brackets. Can you check if husky is initialized and npm run lint / npm run lint:fix works.

Also, there is a cool Typescript trick you can use here:

type ICallbackOrInstance<T> = T | (() => T);

export type IOrbViewSettingsInit<N extends INodeBase, E extends IEdgeBase> = Omit<
  ...
  instances?: {
    simulator?: ICallbackOrInstance<ISimulator>;
  };
}


private _createSimulator(simulator?: ICallbackOrInstance<ISimulator>): ISimulator {
   ...
}

Copy link
Author

Choose a reason for hiding this comment

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

yeah that definitely looks nicer. is there a cleaner way to do the type check instead of

 if(typeof simulator === "function")  return simulator();

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that is the most clean way to check for the function. You could have a util function somewhere. Just as I wrote this comment to give you a link to which place is the best to have a utility function in, I found a function:

https://github.com/memgraph/orb/blob/main/src/utils/type.utils.ts#L87

You already have isFunction type guard check so you can use it here :)


// TODO: Do we keep these
fixNodes() {
this._simulator.fixNodes();
Expand All @@ -579,3 +589,4 @@ export class OrbView<N extends INodeBase, E extends IEdgeBase> implements IOrbVi
this._simulator.releaseNodes();
}
}