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

[Canvas] Feat: Zoom In/Out #38832

Merged
merged 20 commits into from
Jun 25, 2019
Merged

[Canvas] Feat: Zoom In/Out #38832

merged 20 commits into from
Jun 25, 2019

Conversation

cqliu1
Copy link
Contributor

@cqliu1 cqliu1 commented Jun 12, 2019

Summary

Closes #27123.
This adds the ability to zoom in/out of the work space in Canvas.

Jun-13-2019 13-23-00

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cqliu1 cqliu1 force-pushed the feat/zoom-in-out branch from a070ed8 to 00c6a83 Compare June 13, 2019 19:35
@elasticmachine
Copy link
Contributor

💔 Build Failed

@@ -14,6 +14,7 @@ export const getInitialState = path => {
assets: {}, // assets end up here
transient: {
canUserWrite: capabilities.get().canvas.save,
zoomLevel: 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

zoomScale?

@@ -25,3 +25,6 @@ export const DEFAULT_WORKPAD_CSS = '.canvasPage {\n\n}';
export const DEFAULT_ELEMENT_CSS = '.canvasRenderEl{\n\n}';
export const VALID_IMAGE_TYPES = ['gif', 'jpeg', 'png', 'svg+xml'];
export const ASSET_MAX_SIZE = 25000;
export const ZOOM_LEVELS = [0.5, 0.75, 0.9, 1, 1.5, 2, 3, 4];
Copy link
Contributor

Choose a reason for hiding this comment

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

[0.2, 0.25, 0.5, 0.75, 1, 1.5, 2, 3, 4] Allows 5K resolution on 1K screens, FWIW.

@ryankeairns
Copy link
Contributor

ryankeairns commented Jun 13, 2019

@cqliu1 design PR to delineate top and side sections, so that the page will appear to go under them as you zoom in or scroll around:

:octocat: Check out this design PR - https://github.com/cqliu1/kibana/pull/9

Screenshot 2019-06-13 17 15 34

@monfera
Copy link
Contributor

monfera commented Jun 14, 2019

retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

@andreadelrio
Copy link
Contributor

canvas - zoom - openMenu

@cqliu1 what do you think of this?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@cqliu1
Copy link
Contributor Author

cqliu1 commented Jun 17, 2019

canvas - zoom - openMenu

@cqliu1 what do you think of this?

It looks good! I think I'm going to leave the keyboard shortcuts off the menu item for now though. I've talked with @ryankeairns about adding the shortcuts to context menu items so that they align on the right, but I don't think it's possible to add them that way yet.

@cqliu1
Copy link
Contributor Author

cqliu1 commented Jun 18, 2019

@andreadelrio
Screen Shot 2019-06-17 at 2 44 44 PM
I added the context menu for the zoom actions. I added a divider border per @ryankeairns request.

@cqliu1 cqliu1 force-pushed the feat/zoom-in-out branch from 354d03a to 8bb6717 Compare June 18, 2019 05:08
@elasticmachine
Copy link
Contributor

💔 Build Failed

@ryankeairns
Copy link
Contributor

@cqliu1 the zoom icons have been merged on the EUI side, so we'll just have to wait for the next EUI build and subsequent Kibana update.

In the meantime, we could try out the icons if you'd like to store the SVGs in Kibana (EuiIcon now accepts a ref to an SVG file) and later remove them once master is updated. Here are the files:

🗄:arrow_down: zoom-icons.zip

Preview

Screenshot 2019-06-19 08 36 45

@cqliu1 cqliu1 added v7.3.0 v8.0.0 loe:medium Medium Level of Effort labels Jun 19, 2019
@cqliu1
Copy link
Contributor Author

cqliu1 commented Jun 19, 2019

@ryankeairns I'm okay with waiting for the next EUI update as long as it gets into to 7.3.0 before FF.

@cqliu1 cqliu1 marked this pull request as ready for review June 19, 2019 20:04
@cqliu1 cqliu1 requested review from a team as code owners June 19, 2019 20:04
@cqliu1 cqliu1 requested review from ryankeairns and monfera June 19, 2019 20:05
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@monfera monfera left a comment

Choose a reason for hiding this comment

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

Code looks good to me so far 💪 Will approve assuming green CI, the UI steps you discussed with Andrea and Ryan and remaining checklist items, glad to take another look on Friday or next week if the code changes significantly

x: clientX - left,
y: clientY - top,
x: (clientX - left) * (1 / zoomScale),
y: (clientY - top) * (1 / zoomScale),
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 the JIT will optimize it away in any case, we can also do (clientX - left) / zoomScale to save a few bytes

@ryankeairns
Copy link
Contributor

The plan is to start a new EUI release tomorrow (6/21)... stay tuned 📺

@cqliu1 cqliu1 added the review label Jun 25, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

👍 LGTM. This is great, nice work! We can circle back to swap out the icon later.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@cqliu1 cqliu1 merged commit 57869c6 into elastic:master Jun 25, 2019
@cqliu1 cqliu1 deleted the feat/zoom-in-out branch June 25, 2019 20:15
cqliu1 added a commit to cqliu1/kibana that referenced this pull request Jun 25, 2019
* Added zoomScale to transient state

* Added scaling to workpad

* Fixed transform origin

* Fixed mouse coordinate calculation

* Unscaled BorderResizeHandle and RotationHandle

* Added keyboard shortcuts

* Fixed keyboard shortcuts reference

* Updated tests for getPrettyShortcut

* Added tooltip shortcuts

* Added preventDefault to workpad shortcuts

* define interface sections

* Refactor key handler

* Added zoom context menu

* Updated zoom levels

* Fixed ts errors and tests

* Simplified mouse coordinate calculation

* Moved new files to x-pack/legacy/plugins/canvas

* Added TODOs to change icons
cqliu1 added a commit that referenced this pull request Jun 25, 2019
* Added zoomScale to transient state

* Added scaling to workpad

* Fixed transform origin

* Fixed mouse coordinate calculation

* Unscaled BorderResizeHandle and RotationHandle

* Added keyboard shortcuts

* Fixed keyboard shortcuts reference

* Updated tests for getPrettyShortcut

* Added tooltip shortcuts

* Added preventDefault to workpad shortcuts

* define interface sections

* Refactor key handler

* Added zoom context menu

* Updated zoom levels

* Fixed ts errors and tests

* Simplified mouse coordinate calculation

* Moved new files to x-pack/legacy/plugins/canvas

* Added TODOs to change icons
@ambigus9
Copy link

Excelent!!!

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:New Feature New feature not correlating to an existing feature label loe:medium Medium Level of Effort release_note:enhancement review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v7.3.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Canvas] Ability to zoom in/out on a workpad
7 participants