Skip to content

Conversation

@sroy3
Copy link
Contributor

@sroy3 sroy3 commented May 20, 2022

Before:
Screen Shot 2022-05-20 at 11 24 03 AM

Now:
Screen Shot 2022-05-20 at 11 22 42 AM

@sroy3 sroy3 added the product PR that affects product label May 20, 2022
@sroy3 sroy3 self-assigned this May 20, 2022
@sroy3 sroy3 marked this pull request as ready for review May 20, 2022 15:25
@shcheklein
Copy link
Contributor

@sroy3 I like this!

A few comments:

  • top and right spacing does look even? (can be just an optical illusion?). Feels like the right one can be a bit bigger?
  • how does it look like with grips?

@sroy3
Copy link
Contributor Author

sroy3 commented May 20, 2022

@sroy3 I like this!

A few comments:

* top and right spacing does look even? (can be just an optical illusion?). Feels like the right one can be a bit bigger?

It's equal, it's probably because of the label. It's all Vega spacing now since there is no more padding.

* how does it look like with grips?

I made sure to include the gripper when I took the screenshot, but I guess that didn't work. There is just enough space for it.

@mattseddon
Copy link
Contributor

mattseddon commented May 21, 2022

unrelated to this PR

note: from the storybook, right edge on-hover numbers are cropped when plots is zoomed (I remember you explicitly fixed this for non-zoomed)

image

@mattseddon mattseddon enabled auto-merge (squash) May 21, 2022 04:52
@qlty-cloud-legacy
Copy link

Code Climate has analyzed commit 5940473 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 100.0% (85% is the threshold).

This pull request will bring the total coverage in the repository to 96.8% (0.0% change).

View more on Code Climate.

@mattseddon mattseddon merged commit c803254 into main May 21, 2022
@mattseddon mattseddon deleted the packed-plots branch May 21, 2022 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

product PR that affects product

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants