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] Allow to update the viewport fit/zoom level after load #867

Merged
merged 14 commits into from
Nov 16, 2020

Conversation

csouchet
Copy link
Member

@csouchet csouchet commented Nov 6, 2020

Covers #738

⚠️ Merge rendering-diagram.html with navigation-diagram.html --> it will be done later as part of #879

⚠️ we currently always do a mxgraph.zoomActual call prior changing the fit.
With the current fit Center implementation, this seems required. There is also sometime a weird behaviour for the other when we fit the same option multiple times (see #867 (comment)).
This may be managed by #888.

@csouchet csouchet added WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft BPMN diagram usability Something about the way we can interact with BPMN diagrams labels Nov 6, 2020
@csouchet csouchet force-pushed the 738-update_the_viewport_fit_level branch 14 times, most recently from 3909f12 to 84fcd58 Compare November 10, 2020 19:21
@csouchet csouchet added depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first and removed WIP 🚧 Pull request in progress and/or not ready for review. Used in addition to marking it as draft labels Nov 10, 2020
@csouchet csouchet force-pushed the 738-update_the_viewport_fit_level branch 6 times, most recently from f63d9dd to 0a0ed80 Compare November 13, 2020 15:55
@csouchet
Copy link
Member Author

image

@csouchet csouchet force-pushed the 738-update_the_viewport_fit_level branch from c8ecc4a to 1a67d20 Compare November 13, 2020 17:04
@csouchet
Copy link
Member Author

image

@csouchet csouchet force-pushed the 738-update_the_viewport_fit_level branch from 1a67d20 to 2d2ebf2 Compare November 16, 2020 09:30
@csouchet csouchet removed the depends on another PR ⚠️ Pull request depending on another one. The depending must be merged first label Nov 16, 2020
@csouchet csouchet force-pushed the 738-update_the_viewport_fit_level branch from 2d2ebf2 to 223de7e Compare November 16, 2020 09:33
@csouchet csouchet force-pushed the 738-update_the_viewport_fit_level branch from 223de7e to 4dc27a2 Compare November 16, 2020 09:34
@csouchet csouchet marked this pull request as ready for review November 16, 2020 09:44
@csouchet
Copy link
Member Author

csouchet commented Nov 16, 2020

zoomActual only on None

Enregistrement de l’écran 2020-11-16 à 11 33 12

Enregistrement de l’écran 2020-11-16 à 11 34 24

zoomActual everytime

Enregistrement de l’écran 2020-11-16 à 11 32 00

Enregistrement de l’écran 2020-11-16 à 11 36 43

const data = {"zoom":[{"run":1,"LayoutDuration":0.657378,"RecalcStyleDuration":1.038831,"ScriptDuration":3.0980689999999997,"TaskDuration":5.146652},{"run":2,"LayoutDuration":0.663325,"RecalcStyleDuration":1.0146,"ScriptDuration":3.037009,"TaskDuration":5.101386000000001},{"run":3,"LayoutDuration":0.904155,"RecalcStyleDuration":1.313657,"ScriptDuration":3.712731,"TaskDuration":6.398863},{"run":4,"LayoutDuration":0.7462780000000002,"RecalcStyleDuration":1.0772699999999995,"ScriptDuration":3.257247999999999,"TaskDuration":5.5262530000000005},{"run":5,"LayoutDuration":0.678436,"RecalcStyleDuration":1.042242,"ScriptDuration":3.1168819999999986,"TaskDuration":5.229853000000002},{"run":1,"LayoutDuration":0.160489,"RecalcStyleDuration":0.250877,"ScriptDuration":0.905529,"TaskDuration":1.418953},{"run":2,"LayoutDuration":0.14482899999999999,"RecalcStyleDuration":0.213472,"ScriptDuration":0.7874390000000001,"TaskDuration":1.2564440000000001},{"run":3,"LayoutDuration":0.16151400000000005,"RecalcStyleDuration":0.23323000000000005,"ScriptDuration":0.8831800000000001,"TaskDuration":1.393681},{"run":4,"LayoutDuration":0.1670379999999999,"RecalcStyleDuration":0.23873099999999992,"ScriptDuration":0.911724,"TaskDuration":1.443416},{"run":5,"LayoutDuration":0.18802700000000006,"RecalcStyleDuration":0.27260799999999996,"ScriptDuration":0.9736480000000003,"TaskDuration":1.5698979999999993},{"run":3,"LayoutDuration":0.16316,"RecalcStyleDuration":0.25586400000000004,"ScriptDuration":0.91969,"TaskDuration":1.465244},{"run":4,"LayoutDuration":0.16114300000000004,"RecalcStyleDuration":0.23961700000000002,"ScriptDuration":0.9194709999999999,"TaskDuration":1.4454409999999998},{"run":5,"LayoutDuration":0.163356,"RecalcStyleDuration":0.24304100000000006,"ScriptDuration":0.9220190000000001,"TaskDuration":1.4571329999999998},{"run":1,"LayoutDuration":0.173185,"RecalcStyleDuration":0.26366700000000004,"ScriptDuration":0.950573,"TaskDuration":1.494014},{"run":2,"LayoutDuration":0.170918,"RecalcStyleDuration":0.245927,"ScriptDuration":0.892019,"TaskDuration":1.452147},{"run":3,"LayoutDuration":0.18692800000000004,"RecalcStyleDuration":0.2680100000000001,"ScriptDuration":0.964826,"TaskDuration":1.5521020000000005},{"run":4,"LayoutDuration":0.191797,"RecalcStyleDuration":0.27319499999999997,"ScriptDuration":0.96475,"TaskDuration":1.5703000000000005},{"run":5,"LayoutDuration":0.16427199999999997,"RecalcStyleDuration":0.23827600000000015,"ScriptDuration":0.87751,"TaskDuration":1.4108350000000005}],"load":[{"run":1,"LayoutDuration":0.042195,"RecalcStyleDuration":0.035147000000000005,"ScriptDuration":0.021971,"TaskDuration":0.405554},{"run":2,"LayoutDuration":0.023354999999999994,"RecalcStyleDuration":0.035035,"ScriptDuration":0.010188,"TaskDuration":0.339586},{"run":3,"LayoutDuration":0.022187,"RecalcStyleDuration":0.032875,"ScriptDuration":0.010924000000000003,"TaskDuration":0.33594900000000005},{"run":4,"LayoutDuration":0.02291,"RecalcStyleDuration":0.033782999999999994,"ScriptDuration":0.010605999999999997,"TaskDuration":0.33426},{"run":5,"LayoutDuration":0.021916999999999992,"RecalcStyleDuration":0.032103999999999994,"ScriptDuration":0.010824,"TaskDuration":0.3352599999999999},{"run":1,"LayoutDuration":0.037064,"RecalcStyleDuration":0.051883,"ScriptDuration":0.024612,"TaskDuration":0.459436},{"run":2,"LayoutDuration":0.027950000000000003,"RecalcStyleDuration":0.040992,"ScriptDuration":0.010793,"TaskDuration":0.376775},{"run":3,"LayoutDuration":0.029176999999999995,"RecalcStyleDuration":0.04217,"ScriptDuration":0.012958000000000004,"TaskDuration":0.39595499999999995},{"run":4,"LayoutDuration":0.027638999999999997,"RecalcStyleDuration":0.04050200000000001,"ScriptDuration":0.011103999999999996,"TaskDuration":0.37871999999999995},{"run":5,"LayoutDuration":0.027963000000000016,"RecalcStyleDuration":0.040523,"ScriptDuration":0.013883,"TaskDuration":0.40293800000000024},{"run":1,"LayoutDuration":0.033171,"RecalcStyleDuration":0.04804,"ScriptDuration":0.023202,"TaskDuration":0.423848},{"run":2,"LayoutDuration":0.022262999999999998,"RecalcStyleDuration":0.033665,"ScriptDuration":0.010945999999999997,"TaskDuration":0.330909},{"run":3,"LayoutDuration":0.023983000000000004,"RecalcStyleDuration":0.035865999999999995,"ScriptDuration":0.010356000000000004,"TaskDuration":0.34630399999999995},{"run":4,"LayoutDuration":0.021722999999999992,"RecalcStyleDuration":0.03275499999999999,"ScriptDuration":0.011036999999999998,"TaskDuration":0.3456109999999999},{"run":5,"LayoutDuration":0.02229500000000001,"RecalcStyleDuration":0.0335,"ScriptDuration":0.010174999999999997,"TaskDuration":0.32194600000000007},{"run":1,"LayoutDuration":0.029065,"RecalcStyleDuration":0.041504,"ScriptDuration":0.021812,"TaskDuration":0.369482},{"run":2,"LayoutDuration":0.021920999999999996,"RecalcStyleDuration":0.032736,"ScriptDuration":0.009968999999999995,"TaskDuration":0.33135000000000003},{"run":3,"LayoutDuration":0.024193,"RecalcStyleDuration":0.035473000000000005,"ScriptDuration":0.009803000000000006,"TaskDuration":0.337842},{"run":4,"LayoutDuration":0.024107000000000003,"RecalcStyleDuration":0.035344,"ScriptDuration":0.010203999999999998,"TaskDuration":0.34043899999999994},{"run":5,"LayoutDuration":0.023278000000000007,"RecalcStyleDuration":0.034913,"ScriptDuration":0.010440999999999999,"TaskDuration":0.333164}]}
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like there is more than 1 set of 5 runs added here
we supposed to add just one set per feature to not let the data files to BIG in the future

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe the cleaning of this file is missing before adding the information on

Copy link
Contributor

Choose a reason for hiding this comment

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

in fact, data is saved every time tests are launched - we can cover cleaning in #862

Copy link
Member

@tbouffard tbouffard Nov 16, 2020

Choose a reason for hiding this comment

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

I suggest to do the cleaning right now otherwise this will be a mess and we won't know what are the various set of runs. IMHO, doing it in #862 will be too late.
So I suggest that macOS data to be reset to the state prior the implementation of this PR and the dataset recreated.

Comment on lines +14 to +16
<label for="fitOnLoad">Fit on load
<input type="checkbox" id="fitOnLoad" name="fitOnLoad" checked>
</label><br>
Copy link
Member

Choose a reason for hiding this comment

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

this will be reworked as part of #879

src/component/mxgraph/BpmnMxGraph.ts Outdated Show resolved Hide resolved
@csouchet csouchet force-pushed the 738-update_the_viewport_fit_level branch from ae76bf3 to 20be640 Compare November 16, 2020 13:04
@csouchet
Copy link
Member Author

image

@csouchet
Copy link
Member Author

image

@tbouffard
Copy link
Member

Linux performance results

Load time increases a little, I don't know if this is an environment change or an impact of the extra zoomActual

image

Copy link
Member

@tbouffard tbouffard left a comment

Choose a reason for hiding this comment

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

LGTM

@csouchet csouchet merged commit da2750e into master Nov 16, 2020
@csouchet csouchet deleted the 738-update_the_viewport_fit_level branch November 16, 2020 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BPMN diagram usability Something about the way we can interact with BPMN diagrams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants