Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

feature: make above/below colors in filler plugin work with pivoted line charts #12058

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

Merged
merged 2 commits into from
Apr 15, 2025

Conversation

FabTechAT
Copy link
Contributor

above/below colors are ignored when using indexAxis: 'y'

here's how it works now:

grafik

Copy link
Member

@etimberg etimberg left a comment

Choose a reason for hiding this comment

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

Could you add tests for this as well?

@FabTechAT
Copy link
Contributor Author

You mean jasmine tests? I'm not yet familiar with those but I'll see what I can do. Do you think it's sufficient to expect something along the lines of:

expect(dataset.options.fill.target).toBe('origin'); expect(dataset.options.fill.above).toBe('red'); expect(dataset.options.fill.below).toBe('blue'); expect(chart.options.indexAxis).toBe('y');

?

@etimberg
Copy link
Member

Could do a jasmine test, or build an image test similar to the ones in https://github.com/chartjs/Chart.js/pull/12057/files

@FabTechAT
Copy link
Contributor Author

Alright, tests added

…ne charts

resolved conflicts in src/plugins/plugin.filler/filler.drawing.js
@FabTechAT FabTechAT force-pushed the adapt-filler-plugin branch from 98562a3 to e37ac10 Compare April 15, 2025 10:22
@etimberg
Copy link
Member

@LeeLenaleee how do you feel about releasing a 4.4.9 before we merge this?

@LeeLenaleee
Copy link
Collaborator

Yeah that was alos my thought.
Thats why I did not merge these 2 pr's.

I will make a bump pr

@LeeLenaleee LeeLenaleee merged commit 9b1306a into chartjs:master Apr 15, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.