[FEAT] Fit center the viewport on diagram load #836
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
csouchet
added
enhancement
csouchet
force-pushed
the
775-Center_fit_the_viewport_on_diagram_load
branch
11 times, most recently
from
47f34e8
to
25908e0
Compare
csouchet
force-pushed
the
775-Center_fit_the_viewport_on_diagram_load
branch
6 times, most recently
from
65503cc
to
8be617b
Compare
csouchet
force-pushed
the
775-Center_fit_the_viewport_on_diagram_load
branch
from
8be617b
to
ec59649
Compare
csouchet
removed
the
depends on another PR ⚠️
tbouffard
reviewed
Oct 29, 2020
| const width = bounds.width / this.view.scale; | ||
| const height = bounds.height / this.view.scale; | ||
| const scale = Math.min(max, Math.min(clientWidth / width, clientHeight / height)); | ||
| this.cumulativeZoomFactor = scale; |
There was a problem hiding this comment.
I guess this is required for consistency with the zoom code.
Do we need automatic test for that?
There was a problem hiding this comment.
I guess we should have a test who mix zoom & fit
There was a problem hiding this comment.
This will be managed with #844
tbouffard
reviewed
Oct 29, 2020
Co-authored-by: Thomas Bouffard <27200110+tbouffard@users.noreply.github.com>
csouchet
force-pushed
the
775-Center_fit_the_viewport_on_diagram_load
branch
from
a12ec9a
to
ed0dd98
Compare
tbouffard
reviewed
Oct 29, 2020
test/e2e/bpmn.navigation.test.ts
Outdated
| @@ -31,7 +31,7 @@ interface MouseWheelOptions { | |||
| */ | |||
| deltaY?: number; | |||
| } | |||
| interface MouseWithWheel extends Mouse { | |||
| export interface MouseWithWheel extends Mouse { | |||
There was a problem hiding this comment.
❓ we should probably put this definition in a dedicated file to avoid dependencies between test files whereas they have no relation
| viewportCenterY = bounding_box.y + bounding_box.height / 2; | ||
| }); | ||
|
|
||
| it(`ctrl + mouse: initial scale after zoom in and zoom out`, async () => { |
There was a problem hiding this comment.
❓ I don't understand the title
It seems to me that we do fit on load then zoom. Is that correct? if so, the title is not clear IMHO
There was a problem hiding this comment.
Anyway, we will work on this with #844
csouchet
force-pushed
the
775-Center_fit_the_viewport_on_diagram_load
branch
from
dcb4968
to
ed0dd98
Compare
tbouffard
approved these changes
Oct 29, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Covers #775