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

BREAKING drop hardcoded and generated colors #1806

Merged
merged 1 commit into from Feb 10, 2024

Conversation

PowerKiKi
Copy link
Contributor

builtInDefaults and baseColors were dropped. That means that colors
will change to be the default as defined by Chart.js, and we will no
longer generate colors on the fly.

If you need this kind of features, see https://www.chartjs.org/docs/latest/general/colors.html#advanced-color-palettes

As discussed in #1801 (comment)

To be merged afer #1804

@santam85
Copy link
Contributor

santam85 commented Feb 1, 2024

Checks are failing because you only regenerated the macOS (darwin) screenshots. There is a command in the tools folder to regenerate the linux ones using Docker, but it might need some tweaking...

@PowerKiKi
Copy link
Contributor Author

PowerKiKi commented Feb 3, 2024

I'm on linux, so I am actually generating linux images:

image

However, I am having trouble to generate the correct screenshots systematically. They sometimes happens to be 1248px wide, whereas they should all be 600px. Could you give it a try on your side ?

PS: I suspect a timing issue where a animation is sometimes running while it takes a screenshot. But the canvas is styled to be 600px wide, so I don't get how an animation could change that... 🤷

@santam85
Copy link
Contributor

santam85 commented Feb 5, 2024

Very strange this is happening to me too now. Perhaps an update from Chart.js behaviour when rendering responsive: true charts could have caused it?

@santam85
Copy link
Contributor

santam85 commented Feb 9, 2024

@PowerKiKi I ended up having to do substantial changes to the demo app, as it seems the e2e flakyness was caused by the canvas being hidden/animating while the chart was initializing in the lazy loaded tabs. See if the new setup works better for you too, I'll start working on fixing unit tests.

`builtInDefaults` and `baseColors` were dropped. That means that colors
will change to be the default as defined by Chart.js, and we will no
longer generate colors on the fly.

If you need this kind of features, see https://www.chartjs.org/docs/latest/general/colors.html#advanced-color-palettes

As discussed in valor-software#1801 (comment)
@PowerKiKi
Copy link
Contributor Author

I rebased on top of master, and regenerated screenshots. It can now run consistently every single time on my machine. However I see the CI is still failing... I'll see if I can grab the screen shot generated by CI...

@PowerKiKi PowerKiKi force-pushed the powerkiki branch 7 times, most recently from 7833966 to 8fd1b96 Compare February 10, 2024 10:05
@PowerKiKi
Copy link
Contributor Author

I'm getting crazy here. I'm convinced that local e2e is non-flaky. I pushed to CI and it fails. I spend way too much time trying random things with constants failure. And when I'm about to give up, it works ! 🙄

... so I guess it can be merged ... ?

@santam85 santam85 merged commit 7fd1407 into valor-software:master Feb 10, 2024
1 of 2 checks passed
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.

None yet

2 participants