Skip to content
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.

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

feat: add plugin support to ngu-flow #8

Merged
merged 4 commits into from
Feb 23, 2024
Merged

feat: add plugin support to ngu-flow #8

merged 4 commits into from
Feb 23, 2024

Conversation

sheikalthaf
Copy link
Collaborator

@sheikalthaf sheikalthaf commented Feb 14, 2024

Introducing Plugin support, main focus is to tree shake the features and easy to use

export interface FlowPlugin {
  onInit?(data: FlowComponent): void;
  afterInit?(data: FlowComponent): void;
}

In this PR currently two plugins added.

  1. Fit to window
  2. scroll into view

Usage example

<ngu-flow [config]="config">
...
</ngu-flow>
export class DemoComponent {
   plugins = {
      scroll: new ScrollIntoView('1'),
      fitToWindow: new FitToWindow(true),
      arrange: new Arrangements(),
   };
   config: FlowConfig = {
      Arrows: true,
      ArrowSize: 20,
      Plugins: this.plugins,
  };

  focusNode(id: string) {
    this.plugins.scroll.focus(id);
  }

  fitToWindow() {
    this.plugins.fitToWindow.fitToWindow();
  }
  
  autoArrange() {
    this.plugins.arrange.arrange();
  }
}

This is the basic example of the plugin usage.

@santoshyadavdev
Copy link
Member

Can we add some description what we mean by plugin and which plugin we are going to support?

@sheikalthaf
Copy link
Collaborator Author

@santoshyadavdev Yes, for now I'm just exploring how we can add plugin support so that i can be more versatile. Can you throw some idea about general plugin, how they designed?

@sheikalthaf
Copy link
Collaborator Author

@santoshyadavdev can you check this proposal for plugin

Copy link
Member

Choose a reason for hiding this comment

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

_functions generally are used for private functions, can you remove _ if possible.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, it is a private functions only, only for testing purpose i just made in public. Do you have any alternative to access this functions in testcases

projects/flow/src/lib/flow-child.component.ts Outdated Show resolved Hide resolved
projects/flow/src/lib/flow-interface.ts Outdated Show resolved Hide resolved
projects/flow/src/lib/flow.component.ts Show resolved Hide resolved
projects/flow/src/lib/flow.component.ts Show resolved Hide resolved
@@ -61,7 +68,7 @@ describe('Arrangements2', () => {
'7': { x: 660, y: 600, id: '7', deps: ['5'] },
'8': { x: 660, y: 900, id: '8', deps: ['6', '7'] },
};
const actual = Object.fromEntries(arrangements.autoArrange());
const actual = Object.fromEntries(arrangements._autoArrange());
Copy link
Member

Choose a reason for hiding this comment

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

same her lets remove _ from functions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just for testing purpose made it public and added _

Copy link
Member

Choose a reason for hiding this comment

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

yes it should be testes with public methods, call the public methods and check

projects/flow/src/lib/plugins/arrangements.ts Outdated Show resolved Hide resolved
src/app/demo/demo-one.component.ts Outdated Show resolved Hide resolved
@santoshyadavdev
Copy link
Member

Just added sonar cloud to this project as well

@sheikalthaf
Copy link
Collaborator Author

@santoshyadavdev addressed most of the comments and also solved the issues reported by sonar

Copy link

sonarcloud bot commented Feb 20, 2024

Quality Gate Passed Quality Gate passed

Issues
2 New issues

Measures
0 Security Hotspots
No data about Coverage
1.6% Duplication on New Code

See analysis details on SonarCloud

@sheikalthaf
Copy link
Collaborator Author

@santoshyadavdev this PR is ready for review, can you do the final review once

Copy link
Member

@santoshyadavdev santoshyadavdev left a comment

Choose a reason for hiding this comment

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

we should remove the private apis, but let me know if we want to merge this and want to take care of this later.

@sheikalthaf
Copy link
Collaborator Author

@santoshyadavdev for now we are good with the plugin implementation, later I'll change the testcases and avoid public methods.

@santoshyadavdev
Copy link
Member

Cool let's merge it

@sheikalthaf
Copy link
Collaborator Author

@santoshyadavdev Need your approval to merge

@sheikalthaf sheikalthaf merged commit 66bc6f7 into main Feb 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants