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

Jump to bottom

[PF-48] - Builder integration changes #5

Merged

tuplle merged 31 commits into next from PF-48

Nov 5, 2023

Contributor

mazarijuraj commented Oct 5, 2023 •

edited by jira bot

Loading

Description

Code refactor and changes made by integration into builder. Example project should be updated in near future to reflect builder implementation changes (marked with // TODO: PF-48 comments).

Implements PF-48

mazarijuraj and others added 25 commits

October 26, 2022 16:04


          [PF-47] - Replace panzoom with @panzoom/panzoom

ec2852e

- add support for svg grid bakcground


          [PF-47] - Replace panzoom with @panzoom/panzoom

4e8439a

- replace panzoom with @panzoom/panzoom


          [PF-47] - Replace panzoom with @panzoom/panzoom

fc9224c

- fix import in grid configuration
- add option for cursor type
- remove unused css class


          [PF-48] - Builder integration changes

02efbd9

- add isActive function for activable classes
- rename functions for better readability


          [PF-48] - Builder integration changes

1d817d0

- rename functions for better readability


          [PF-48] Builder integration changes

ca17e3e

- fix lasso tool
- fix moving of canvas on each mouse down + move event


          [PF-48] Builder integration changes

012af2f

- add select tool
- add event propagation


          [PF-48] Builder integration changes

e8fdc38

- fix arc removal


          [PF-48] Builder integration changes

0c1da2f

- decoupling library


          [PF-48] Builder integration changes

9c6114d

- decouple library stage 1


          [PF-48] Builder integration changes

840aa32

- decouple library stage 2 with modeler necessary functions


          [PF-48] Builder integration changes

1f3af2d

- changes to behavior of marking nodes of place


          [PF-48] - Builder integration changes

b62116b

- add comments for future rework


          [PF-48] Builder integration changes

633548a

- push old


          Merge remote-tracking branch 'origin/PF-48' into PF-48

f49ba5d


          [PF-48] - Builder integration changes

5e7c5ce

- merge canvas configuration service with canvas service


          [PF-48] - Builder integration changes

896bd00

- fix problem with marker ends of arcs not highlighted on hover


          [PF-48] - Builder integration changes

0c552b0

- add function to get all child elements of canvas element


          [PF-48] - Builder integration changes

ef312a6

- make Container not abstract to use in Canvas
- add containers for arcs, places and transition to fix ordering in svg element
- add static methods for creating petriflow.svg elements without instantiating petri.svg elements


          [PF-48] - Builder integration changes

10362a0

- fix background position i svg DOM tree


          [PF-48] - Builder integration changes

3b59904

- fix background position of labels


          [PF-48] - Builder integration changes

8894aae

- fix background position of labels


          [PF-48] - Builder integration changes

da7d44c

- fix empty label showing background
- add updateLine method to repaint arc line points


          [PF-48] - Builder integration changes

93b72cb

- remove events on mouse over/out
- add comments for future refactor


          [PF-48] - Builder integration changes

adb5889

- remove radius from canvas config, use size instead
- add missing abstract deactivate method to petriflow canvas element

mazarijuraj requested review from tuplle and minop

October 5, 2023 09:20

mazarijuraj self-assigned this

pull-request-quantifier-deprecated bot added the Extra Large label

minop approved these changes

View reviewed changes

projects/example-app/src/app/app.component.html

@@ @@ -1,12 +1,15 @@ @@
               <mat-toolbar fxLayout="row" class="app-container mat-toolbar mat-toolbar-single-row">
                   <mat-toolbar-row fxFlex>
                       <mat-icon id="netgrif-icon" svgIcon="netgrif_logo"></mat-icon>
-                      <mat-button-toggle-group [value]="_mode" fxLayoutAlign="start end">
+                      <mat-button-toggle-group [(ngModel)]="_mode" fxLayoutAlign="start end">

minop Oct 5, 2023

why ngModel? Can be done with FormGroup https://angular.io/api/forms/FormGroup

projects/example-app/src/app/petriflow-control-set/services/control-petriflow-canvas.service.ts

+              })
+              export class ControlPetriflowCanvasService extends PetriflowCanvasService {
+                  private arcTypes = ['regular', 'reset', 'inhibitor', 'read'];

minop Oct 5, 2023

could reference the enum values? Why separate?

projects/example-app/src/app/petriflow-control-set/services/control-petriflow-canvas.service.ts

+                  selectAll() {
+                      this.petriflowElementsCollection.all.forEach(element => {
+                          element.select();

minop Oct 5, 2023

setSelectedByRectangleEnclosure method does setSelected(true) and activate(). Why are there differences?

projects/example-app/src/app/petriflow-control-set/services/control-petriflow-canvas.service.ts

+                      this.moveBreakpoint(e);
+                      if (this.mouseDown && this.mode === CanvasMode.LASSO) {
+                          if (!this.canvas) {
+                              throw new Error('SVG canvas for petriflow objects doesn\'t exists!');

minop Oct 5, 2023

the \ could be dropped if " or ` was used do delimit the string

projects/example-app/src/app/petriflow-control-set/services/control-petriflow-canvas.service.ts

+                  };
+                  addTransitionEvents(petriflowTransition: PetriflowTransition): void {
+                      // @ts-ignore

minop Oct 5, 2023

why is the @ts-ignore needed?

projects/example-app/src/app/petriflow-control-set/services/control-petriflow-canvas.service.ts

+                      });
+                  }
+                  // @ts-ignore

minop Oct 5, 2023

why @ts-ignore?

projects/example-app/src/app/petriflow-control-set/services/control-petriflow-canvas.service.ts

+                      }
+                      if (this._source.canvasElement instanceof Place) {
+                          switch (type) {
+                              case 'regular': {

minop Oct 5, 2023

why not enum values?

projects/example-app/src/app/petriflow-control-set/services/control-petriflow-canvas.service.ts

+                      return undefined;
+                  }
+                  // @ts-ignore

minop Oct 5, 2023

why @ts-ignore?

projects/example-app/src/app/petriflow-control-set/services/control-petriflow-canvas.service.ts

+                          }
+                      }
+                      if (this._mode === CanvasMode.SELECT) {
+                      }

minop Oct 5, 2023

empty if

mazarijuraj added 6 commits

October 10, 2023 13:08


          [PF-48] - Builder integration changes

f44011f

- fix missing svg element ids


          [PF-48] - Builder integration changes

965cbb4

- fix missing svg element ids


          [PF-48] - Builder integration changes

0d9206b

- fix missing svg container ids


          [PF-48] - Builder integration changes

f8bfd0e

- fix missing svg container ids


          [PF-48] - Builder integration changes

fb41fb5

- fix id label


          [PF-48] - Builder integration changes

50442a6

- fix label background incorrect size

pull-request-quantifier-deprecated bot commented Oct 23, 2023

This PR has 1981 quantified lines of changes. In general, a change size of upto 200 lines is ideal for the best PR experience!

Quantification details

Label      : Extra Large
Size       : +1148 -833
Percentile : 100%

Total files changed: 60

Change summary by file extension:
.sh : +26 -0
.json : +8 -5
.html : +22 -4
.scss : +2 -5
.ts : +1090 -819

Change counts above are quantified counts, based on the PullRequestQuantifier customizations.

Why proper sizing of changes matters

Optimal pull request sizes drive a better predictable PR flow as they strike a
balance between between PR complexity and PR review overhead. PRs within the
optimal size (typical small, or medium sized PRs) mean:

Fast and predictable releases to production:
- Optimal size changes are more likely to be reviewed faster with fewer
  iterations.
- Similarity in low PR complexity drives similar review times.
Review quality is likely higher as complexity is lower:
- Bugs are more likely to be detected.
- Code inconsistencies are more likely to be detected.
Knowledge sharing is improved within the participants:
- Small portions can be assimilated better.
Better engineering practices are exercised:
- Solving big problems by dividing them in well contained, smaller problems.
- Exercising separation of concerns within the code changes.

What can I do to optimize my changes

Use the PullRequestQuantifier to quantify your PR accurately
- Create a context profile for your repo using the context generator
- Exclude files that are not necessary to be reviewed or do not increase the review complexity. Example: Autogenerated code, docs, project IDE setting files, binaries, etc. Check out the Excluded section from your prquantifier.yaml context profile.
- Understand your typical change complexity, drive towards the desired complexity by adjusting the label mapping in your prquantifier.yaml context profile.
- Only use the labels that matter to you, see context specification to customize your prquantifier.yaml context profile.
Change your engineering behaviors
- For PRs that fall outside of the desired spectrum, review the details and check if:
  - Your PR could be split in smaller, self-contained PRs instead
  - Your PR only solves one particular issue. (For example, don't refactor and code new features in the same PR).

How to interpret the change counts in git diff output

One line was added: +1 -0
One line was deleted: +0 -1
One line was modified: +1 -1 (git diff doesn't know about modified, it will
interpret that line like one addition plus one deletion)
Change percentiles: Change characteristics (addition, deletion, modification)
of this PR in relation to all other PRs within the repository.

Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email)
Customize PullRequestQuantifier for this repository.

Member

tuplle commented Nov 5, 2023

All comments are about the example-app which is subject of planned refactor in the next release.

tuplle merged commit bf24e90 into next

4 of 5 checks passed

tuplle deleted the PF-48 branch

November 5, 2023 17:19

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels