Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix: remove 1px padding and re-enable autoscale snapshot test #6271

Merged
merged 2 commits into from
Feb 2, 2023
Merged

Conversation

ozyx
Copy link
Contributor

@ozyx ozyx commented Feb 2, 2023

Closes #6267

Describe your changes:

Removes a seemingly unnecessary 1px padding to plot legend items that was squishing our plot and causing our snapshot tests to fail. Re-enables the autoscale snapshot tests.

Here is the plot legend item without 1px padding:
image

..and with:
image

@charlesh88 thoughts? is this ok to remove or should we regenerate snapshots?

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Is this change backwards compatible? For example, developers won't need to change how they are calling the API or how they've extended core plugins such as Tables or Plots.

Author Checklist

  • Changes address original issue?
  • Tests included and/or updated with changes?
  • Command line build passes?
  • Has this been smoke tested?
  • Testing instructions included in associated issue OR is this a dependency/testcase change?

Reviewer Checklist

  • Changes appear to address issue?
  • Reviewer has tested changes by following the provided instructions?
  • Changes appear not to be breaking changes?
  • Appropriate automated tests included?
  • Code style and in-line documentation are appropriate?
  • Has associated issue been labelled unverified? (only applicable if this PR closes the issue)
  • Has associated issue been labelled bug? (only applicable if this PR is for a bug fix)

@ozyx ozyx added this to the Target:2.2.0 milestone Feb 2, 2023
Copy link
Contributor

@charlesh88 charlesh88 left a comment

Choose a reason for hiding this comment

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

LGTM!

@codecov
Copy link

codecov bot commented Feb 2, 2023

Codecov Report

Merging #6271 (e84e480) into master (c1e8c79) will decrease coverage by 0.27%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6271      +/-   ##
==========================================
- Coverage   55.42%   55.15%   -0.27%     
==========================================
  Files         607      607              
  Lines       26090    26090              
  Branches     2376     2376              
==========================================
- Hits        14460    14391      -69     
- Misses      10968    11031      +63     
- Partials      662      668       +6     
Flag Coverage Δ *Carryforward flag
e2e-ci 39.49% <ø> (ø) Carriedforward from c1e8c79
e2e-full 51.23% <ø> (ø) Carriedforward from c1e8c79
e2e-stable 53.97% <ø> (+0.13%) ⬆️
unit 49.71% <ø> (-0.47%) ⬇️

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
.../telemetryTable/components/table-configuration.vue 0.00% <0.00%> (-43.25%) ⬇️
src/ui/inspector/InspectorViews.vue 64.28% <0.00%> (-28.58%) ⬇️
src/ui/components/ObjectLabel.vue 0.00% <0.00%> (-24.00%) ⬇️
...c/plugins/telemetryTable/components/sizing-row.vue 38.88% <0.00%> (-11.12%) ⬇️
src/ui/components/ObjectPath.vue 80.95% <0.00%> (-9.53%) ⬇️
...lemetryTable/components/table-footer-indicator.vue 26.41% <0.00%> (-9.44%) ⬇️
src/plugins/telemetryTable/components/table.vue 41.19% <0.00%> (-4.61%) ⬇️
src/ui/components/ObjectView.vue 47.91% <0.00%> (-4.17%) ⬇️
src/ui/inspector/Inspector.vue 76.47% <0.00%> (-2.95%) ⬇️
src/ui/layout/BrowseBar.vue 44.31% <0.00%> (-2.28%) ⬇️
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1e8c79...e84e480. Read the comment docs.

@ozyx ozyx merged commit 800062d into master Feb 2, 2023
@ozyx ozyx deleted the mct6267 branch February 2, 2023 23:50
ozyx added a commit that referenced this pull request Feb 8, 2023
…napshot test

* style: remove 1px padding from plot legend item

* test: re-enable autoscale snapshot test
akhenry pushed a commit that referenced this pull request Feb 9, 2023
…napshot test (#6303)

* style: remove 1px padding from plot legend item
* test: re-enable autoscale snapshot test
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[e2e] Re-enable snapshot tests
3 participants