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

Extract markers into plugin #1687

Merged
merged 16 commits into from
Sep 4, 2024
Merged

Conversation

illetid
Copy link
Contributor

@illetid illetid commented Aug 29, 2024

Type of PR: enhancement

  • N/A Addresses an existing issue: fixes #
  • Includes tests
  • Documentation update

Overview of change:
Moved a series markers into a built-in plugin to enable tree shaking

Copy link

github-actions bot commented Aug 29, 2024

size-limit report 📦

Path Size
ESM 43.39 KB (+0.73% 🔺)
ESM createChart 40.3 KB (-3.6% 🔽)
ESM createChartEx 39.08 KB (-3.63% 🔽)
ESM Standalone 44.91 KB (+0.9% 🔺)
Standalone 44.87 KB (+0.9% 🔺)
Plugin: Text Watermark 1.87 KB (+4.48% 🔺)
Plugin: Image Watermark 1.7 KB (+5.33% 🔺)
Plugin: Series Markers 3.89 KB (+100% 🔺)

Copy link
Contributor

@SlicedSilver SlicedSilver left a comment

Choose a reason for hiding this comment

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

  • Could we also add a short documentation page for the plugin?
  • I know the graphics tests will fail because the interface changed. Did you manage to run the tests locally before the changes, save those images somewhere, and then run again with the current changes? If so, could you check that the screenshots are the same? It's a bit of pain to manually do this I guess. Downside of our current screenshot testing system.

.size-limit.js Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
src/model/autoscale-info-impl.ts Show resolved Hide resolved
src/plugins/series-markers/primitive.ts Outdated Show resolved Hide resolved
src/plugins/series-markers/primitive.ts Outdated Show resolved Hide resolved
src/plugins/series-markers/types.ts Outdated Show resolved Hide resolved
src/plugins/series-markers/utils.ts Outdated Show resolved Hide resolved
src/plugins/pane-primitive-wrapper.ts Outdated Show resolved Hide resolved
src/plugins/image-watermark/primitive.ts Outdated Show resolved Hide resolved
website/docs/migrations/from-v4-to-v5.md Outdated Show resolved Hide resolved
tests/type-checks/series-markers.ts Outdated Show resolved Hide resolved
tests/type-checks/non-time-based-custom-series.ts Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@SlicedSilver
Copy link
Contributor

  • There is a failing test within the 'coverage' e2e suite.
  • Could you check the graphics e2e tests and ensure that all the tests are generating a 2. test.png and that the image looks correct. You might need to run the tests locally on golden (v5-candidate) with these line removed: to get the correct baseline images and store them somewhere. Then run the e2e tests again but this time on this branch (and that line added back in), and then manually compare the images. Perhaps by having a script get the matching images for the baseline folder and the tests folder → place them into a new directory with naming like: series-markers_test.png and series-markers_golden.png and then you can quickly go through the images one by one and quickly notice if anything looks different. (mac quicklook and arrow keys work well for this)

@illetid
Copy link
Contributor Author

illetid commented Sep 4, 2024

  • There is a failing test within the 'coverage' e2e suite.
  • Could you check the graphics e2e tests and ensure that all the tests are generating a 2. test.png and that the image looks correct. You might need to run the tests locally on golden (v5-candidate) with these line removed:
    to get the correct baseline images and store them somewhere. Then run the e2e tests again but this time on this branch (and that line added back in), and then manually compare the images. Perhaps by having a script get the matching images for the baseline folder and the tests folder → place them into a new directory with naming like: series-markers_test.png and series-markers_golden.png and then you can quickly go through the images one by one and quickly notice if anything looks different. (mac quicklook and arrow keys work well for this)

I've checked all the screenshots. Fixed one small issue with margins that I've spotted in progress, and failing coverage tests are also fixed, I used wrong interface as returned type and ts-transformer renamed properties that should be public.
attached zip with tests results and added in each folder image from reference run to check with
devicePixelRatio=2.00.zip

Copy link
Contributor

@SlicedSilver SlicedSilver left a comment

Choose a reason for hiding this comment

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

:shipit:

@illetid illetid merged commit 221360e into v5-candidate Sep 4, 2024
17 of 33 checks passed
@illetid illetid deleted the extract-markers-into-plugin branch September 4, 2024 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants