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.

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

[Canvas][Layout Engine] Annotation showing node rotation angle #30702

Merged
merged 2 commits into from
Feb 14, 2019
Merged

[Canvas][Layout Engine] Annotation showing node rotation angle #30702

merged 2 commits into from
Feb 14, 2019

Conversation

monfera
Copy link
Contributor

@monfera monfera commented Feb 11, 2019 •
edited
Loading

Shows rotation degree. This change also enabled that instead of 45 degree increments, the snapping is done in 15 degree increments. (It still works such that snapping threshold is tighter if the cursor is close to the shape origin but less restrictive if it's far from the origin)

Completes some usability items here and is rebased on other things in the backed-up PR pipeline, so see the last commit only.

rotate deg

@ryankeairns it'd benefit from a bit of design and CSS implementation, I believe :-) Eg. black number won't be seen on a dark background, so maybe it could get a backing color like in eg. Ai.

@monfera monfera requested review from a team as code owners February 11, 2019 18:59
@monfera monfera force-pushed the layout-usability-improvements-2 branch from 7d2b45e to 1cd80c7 Compare February 11, 2019 19:17
@monfera monfera self-assigned this Feb 11, 2019
@monfera monfera added review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas labels Feb 11, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-canvas

@elastic elastic deleted a comment from elasticmachine Feb 11, 2019
@monfera monfera force-pushed the layout-usability-improvements-2 branch from 1cd80c7 to 3db3604 Compare February 12, 2019 14:58
@alexfrancoeur
Copy link

@monfera is this good to test or would you prefer I wait?

@monfera
Copy link
Contributor Author

monfera commented Feb 12, 2019

@alexfrancoeur thanks, it'd be good to test, I indicate it on my PRs with the blue review label, and no WIP in the title

@monfera monfera force-pushed the layout-usability-improvements-2 branch from 3db3604 to 9c52e0f Compare February 13, 2019 08:32
@elastic elastic deleted a comment from elasticmachine Feb 13, 2019
@elastic elastic deleted a comment from elasticmachine Feb 13, 2019
@elastic elastic deleted a comment from elasticmachine Feb 13, 2019
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

ryankeairns
ryankeairns reviewed Feb 13, 2019
View reviewed changes
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.

The functionality is great!
I've created a PR against this branch with some design changes - #30950

I'm not sure if you can merge it in or just make the changes on your local branch as they are quite minor.

Thanks!

@alexfrancoeur
Copy link

LGTM. I tested a few different scenarios out manually and didn't run into any issues. 15 degree intervals look nice, single degree intervals work as expected as well. As far as the styling of labeling goes I have two questions and may need to defer to @ryankeairns here.

  1. What should the experience be like in dark mode for labels? In light mode, it's black text but in dark mode it's grey. Does it make sense to keep the coloring consistent as the background is totally dependent on what the user defined, not necessarily Kibana's UI.

screen shot 2019-02-13 at 10 54 18 am

  1. Should we just make the text pink to keep consistent with the snapping components?

@ryankeairns
Copy link
Contributor

ryankeairns commented Feb 13, 2019 •
edited
Loading

@alexfrancoeur I've linked to a PR in my previous comment that styles the annotation tooltip like an EUI tooltip. This way it will work in both light and dark mode as seen below (i.e. white text on dark background in both cases; same as tooltips across Kibana):

Dark mode

screenshot 2019-02-13 08 33 47 3

Light mode

screenshot 2019-02-13 08 32 37

cqliu1
cqliu1 requested changes Feb 13, 2019
View reviewed changes
Copy link
Contributor

@cqliu1 cqliu1 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM 👍 It's such a nice improvement to see the rotate angle.

I just came across one issue. If you're rotating an element and drag the mouse outside of the workpad, it gets hidden under the workpad header and the sidebar. Is there anyway we can increase the z-index and have it display on top?

feb-13-2019 11-22-39

This isn't that big of a visual bug and isn't a blocker for this PR.

@monfera
Copy link
Contributor Author

monfera commented Feb 13, 2019

@cqliu1 thanks, will take a look, there's a small, expected merge conflict I need to solve due to the merged other PRs. If it's easy I'll push a change, although maybe it can even be done with a CSS change? Will see in a bit

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@monfera monfera force-pushed the layout-usability-improvements-2 branch from 8fc0425 to a65820d Compare February 13, 2019 19:22
ryankeairns
ryankeairns approved these changes Feb 13, 2019
View reviewed changes
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

@monfera monfera force-pushed the layout-usability-improvements-2 branch from a65820d to 3e44998 Compare February 13, 2019 21:01
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@monfera monfera force-pushed the layout-usability-improvements-2 branch from 3e44998 to 95040ac Compare February 14, 2019 06:27
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@monfera monfera merged commit 0e7a735 into master Feb 14, 2019
@monfera monfera mentioned this pull request Feb 14, 2019
Merged
monfera added a commit to monfera/kibana that referenced this pull request Feb 14, 2019
@monfera
[Canvas][Layout Engine] Annotation showing node rotation angle (elast…
2c0149d 
…ic#30702)

* Feat: angle rotation annotation (in degrees) added
* Style annotation like EUI tooltip (elastic#30950)
@monfera monfera mentioned this pull request Feb 14, 2019
Merged
monfera added a commit to monfera/kibana that referenced this pull request Feb 14, 2019
@monfera
[Canvas][Layout Engine] Annotation showing node rotation angle (elast…
35e8522 
…ic#30702)

* Feat: angle rotation annotation (in degrees) added
* Style annotation like EUI tooltip (elastic#30950)
@monfera monfera mentioned this pull request Feb 14, 2019
Merged
monfera added a commit to monfera/kibana that referenced this pull request Feb 14, 2019
@monfera
[Canvas][Layout Engine] Annotation showing node rotation angle (elast…
01b207c 
…ic#30702)

* Feat: angle rotation annotation (in degrees) added
* Style annotation like EUI tooltip (elastic#30950)
@monfera monfera deleted the layout-usability-improvements-2 branch February 14, 2019 08:11
monfera added a commit that referenced this pull request Feb 14, 2019
@monfera
[Canvas][Layout Engine] Annotation showing node rotation angle (#30702)…
c83cd96 
… (#31082)

* Feat: angle rotation annotation (in degrees) added
* Style annotation like EUI tooltip (#30950)
monfera added a commit that referenced this pull request Feb 14, 2019
@monfera
[Canvas][Layout Engine] Annotation showing node rotation angle (#30702)…
fc16e46 
… (#31083)

* Feat: angle rotation annotation (in degrees) added
* Style annotation like EUI tooltip (#30950)
monfera added a commit that referenced this pull request Feb 14, 2019
@monfera
[Canvas][Layout Engine] Annotation showing node rotation angle (#30702)…
cf32861 
… (#31084)

* Feat: angle rotation annotation (in degrees) added
* Style annotation like EUI tooltip (#30950)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexfrancoeur alexfrancoeur alexfrancoeur approved these changes

@cqliu1 cqliu1 cqliu1 approved these changes

@ryankeairns ryankeairns ryankeairns approved these changes

Assignees

@monfera monfera

Labels
review Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@monfera @elasticmachine @alexfrancoeur @ryankeairns @cqliu1