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

provide option to display totals on top of each bar #5375

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

wilhelmhb
Copy link

This PR aims at allowing users to display the total of each bar on top of it (bottom of it for negative values).

TODO:

@wilhelmhb
Copy link
Author

I haven't yet been able to run the tests, probably a browser/configuration issue. Thus I haven't started on the jasmine tests. I can provide a few test cases if needed.
I don't really see where the documentation is written, but if someone points me in the right direction, I'll be glad to add a few examples.

Copy link
Contributor

@archmoj archmoj left a comment

Choose a reason for hiding this comment

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

Thanks very much for the PR.
Before adding tests, I suggest you sync the master branch of fork with the latest changes of plotly/plotly.js/master; then git merge upstream/master into this branch.
Please find my other comments below:

src/traces/bar/layout_attributes.js Show resolved Hide resolved
src/traces/bar/layout_attributes.js Show resolved Hide resolved
src/traces/bar/plot.js Outdated Show resolved Hide resolved
src/traces/bar/plot.js Outdated Show resolved Hide resolved
@@ -63,5 +63,17 @@ module.exports = {
'Sets the gap (in plot fraction) between bars of',
'the same location coordinate.'
].join(' ')
}
},
displaytotal: {
Copy link
Contributor

Choose a reason for hiding this comment

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

displaytotal is too general while this attribute lives in layout.
How about barshowtotal?

Copy link
Author

Choose a reason for hiding this comment

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

Much better indeed.
Should I also rename totaltemplate to bartotaltemplate, so that each bar specific option starts with bar ? Or do we want to reuse totaltemplate for area charts as well ?

src/traces/bar/layout_defaults.js Outdated Show resolved Hide resolved
@archmoj archmoj added status: reviewable community community contribution feature something new labels Jan 6, 2021
@wilhelmhb wilhelmhb changed the title [WIP][Bars] Option to display totals on top of each bar [Bars] Option to display totals on top of each bar Mar 9, 2021
@wilhelmhb
Copy link
Author

Any update on this ?

@nicolaskruchten
Copy link
Contributor

Thanks for this PR! Our team is currently working hard on finishing up the 2.0 release of Plotly.js which explains our radio-silence for the past while, and I apologize for not communicating more clearly about it. This PR should probably land in 2.1, which we expect to come out in April. I'm sorry for the long timelines here :)

@mysheepb
Copy link

I am interested in using this feature. Any update?

@vovavili
Copy link

@nicolaskruchten Hello! I am interested in using this feature. Now that we're reaching April 2022 and plotly 2.11, are there any updates on this PR?

@archmoj
Copy link
Contributor

archmoj commented Mar 23, 2022

@vovavili could you please fetch upstream/master and merge it into this branch?

@hjc0826
Copy link

hjc0826 commented May 23, 2022

I am interested in using this feature. Any update?

@wilhelmhb
Copy link
Author

@nicolaskruchten @archmoj I've just merged upstream/master in this branch
I removed a typo, and added an example in test/image/mocks/bar_stackrelative_negative_total.json, available when you run npm start, then lookup the filename in the mocks search field.

I personally have a weird result on my machine. However, as I have the same result when using any text with textposition : outside, I'm assuming this is an issue with plotly.js, which should be a separate matter.

I'm still unable to run the jasmine tests on my machine.

Hoping this can still make it to 2.13

@hadarby
Copy link

hadarby commented Jul 31, 2022

Hi there is any update?

barshowtotal: {
valType: 'boolean',
dflt: false,
role: 'style',
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no longer role needed for attributes like this one.
Please remove the role: 'style', line.

totaltemplate: {
valType: 'string',
dflt: '%{total}',
role: 'style',
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no longer role needed for attributes like this one.
Please remove the role: 'style', line.

editType: 'plot',
description: 'Display the total value of each bar.'
},
totaltemplate: {
Copy link
Contributor

Choose a reason for hiding this comment

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

We may want this attribute or a similar one to control hover.
It may be better to have

stackbartotal: {
  texttemplate: ...,
  hovertemplate: ...
}

And there is no need for barshowtotal as it could be enabled by texttemplate.

@archmoj
Copy link
Contributor

archmoj commented Nov 7, 2022

Thanks @wilhelmhb for the great PR.
My apologies for the late review.
Kindly find my comments regarding the new attributes.
Before adding new tests, I also suggest that you possibly pull upstream/master and merge it to your branch.
@alexcjohnson it would be great if you also provide a review. Thank you!

@Pampelmops
Copy link

October 2023, I could really use this feature. Thank you.

@futurejohn
Copy link

@archmoj @alexcjohnson How is the review going? It would be really helpful to display totals for bar charts.

@gvwilson gvwilson self-assigned this May 27, 2024
@archmoj
Copy link
Contributor

archmoj commented May 28, 2024

Would you please fetch upstream/master, merge it into this branch and resolve conflicts 🙏
We are hoping to include this feature.
Thank you!

@gvwilson gvwilson removed their assignment Aug 2, 2024
@gvwilson gvwilson changed the title [Bars] Option to display totals on top of each bar Option to display totals on top of each bar Aug 8, 2024
@gvwilson gvwilson changed the title Option to display totals on top of each bar provide option to display totals on top of each bar Aug 9, 2024
@gvwilson gvwilson added the P2 considered for next cycle label Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution feature something new P2 considered for next cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.