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

Ensure that controllers derived from the bar controller work correct in stacked charts #9587

Merged
merged 4 commits into from Sep 4, 2021

Conversation

shubham242k
Copy link
Contributor

@shubham242k shubham242k commented Aug 26, 2021

Issue Fixes #9569 and fixes #9595

What was the Issue?

when deriving a new chart type by extending BarController and using the default functionality of the default bar chart type, the bars are overlapping in a small space.

Unexpected behaviour: https://codesandbox.io/s/unexpected-behaviour-yiy8y?file=/src/index.js
image

Expected behaviour:
image

Changes Made

changes the argument passed in getMatchingVisibleMetas

const metas = scale.getMatchingVisibleMetas('bar');

from 'bar' to meta._type .

getMatchingVisibleMetas returns the datasets that are visible and have type equals to meta._type

UPDATE
Also changed the argument passed in 'getMatchingVisibleMetas' at

for (const meta of vScale.getMatchingVisibleMetas('bar').reverse()) {

@CatchABus
Copy link
Contributor

CatchABus commented Aug 27, 2021

@shubham242k There is also this one: https://github.com/chartjs/Chart.js/blob/master/src/core/core.datasetController.js#L132

UPDATED: This is a different issue, therefore it's not related to this PR.

Copy link

@KLN888 KLN888 left a comment

Choose a reason for hiding this comment

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

This is working fine now.

KLN888
KLN888 previously approved these changes Aug 28, 2021
Copy link

@KLN888 KLN888 left a comment

Choose a reason for hiding this comment

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

It's fine now anyone can check again

@shubham242k shubham242k changed the title change parameter of functions Change parameter of functions Aug 28, 2021
src/controllers/controller.bar.js Outdated Show resolved Hide resolved
CatchABus
CatchABus previously approved these changes Aug 29, 2021
@CatchABus
Copy link
Contributor

I tested these changes and they fix:

  1. Bars overlapping
  2. Styling properties not being properly applied on stacked bars

src/controllers/controller.bar.js Outdated Show resolved Hide resolved
src/controllers/controller.bar.js Show resolved Hide resolved
Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

A test would be good, but I'm not sure how that could be done.

@shubham242k
Copy link
Contributor Author

shubham242k commented Sep 3, 2021

A test would be good, but I'm not sure how that could be done.

I have tried to add the test case where I can put canvas inside a small size div for testing overlapping issues , But did not find a way

@shubham242k shubham242k closed this Sep 3, 2021
@shubham242k shubham242k reopened this Sep 3, 2021
@etimberg etimberg added this to the Version 3.6.0 milestone Sep 4, 2021
@etimberg etimberg changed the title Change parameter of functions Ensure that controllers derived from the bar controller work correct in stacked charts Sep 4, 2021
@etimberg etimberg merged commit 4af9851 into chartjs:master Sep 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants