Skip to content

Conversation

@yoshiokatsuneo
Copy link
Contributor

@yoshiokatsuneo yoshiokatsuneo commented Mar 6, 2025

What type of PR is this?

  • Bug Fix

Description

After upgraded plotly.js to version2.0, "show data labels" on bar chart does not works but data label is always show.
This happens because plotly.js's "textposition" default value was changed from "none" to "auto".

This PR fixes it by explicitly setting "textposition" to "none" or "inside".

How is this tested?

  • Manually

I confirmed that data labels is shown only when "show data labels" is checked.

Test Query:

SELECT 'a' AS name, 1 as value union all
SELECT 'b' AS name, 3 as value union all
SELECT 'c' AS name, 2 as value
image image

@yoshiokatsuneo yoshiokatsuneo marked this pull request as ready for review March 6, 2025 06:28
@yoshiokatsuneo yoshiokatsuneo force-pushed the fix/bar_chart_show_data_labels_3 branch from d849075 to 3a3fff1 Compare March 6, 2025 06:34
@yoshiokatsuneo yoshiokatsuneo force-pushed the fix/bar_chart_show_data_labels_3 branch from 3a3fff1 to 7c32e7c Compare March 6, 2025 08:53
@yoshiokatsuneo yoshiokatsuneo changed the title Fix to make "show data labels" works (one line PR) Fix to make "show data labels" works Mar 6, 2025
@yoshiokatsuneo yoshiokatsuneo changed the title (one line PR) Fix to make "show data labels" works Fix to make "show data labels" works Mar 6, 2025
@yoshiokatsuneo yoshiokatsuneo changed the title Fix to make "show data labels" works (2 lines PR) Fix to make "show data labels" works Mar 8, 2025
@yoshiokatsuneo
Copy link
Contributor Author

yoshiokatsuneo commented Mar 15, 2025

@eradman

If possible, can you review this PR fixing the issue related to plotly.js 2.0 ( #7359 ) ? The same issue is reported as #7372 .
This PR is 2-lines of simple code, I think.

Copy link
Collaborator

@eradman eradman left a comment

Choose a reason for hiding this comment

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

Fix works. Thanks @yoshiokatsuneo!

@eradman eradman merged commit 353776e into getredash:master Mar 17, 2025
11 checks passed
@yoshiokatsuneo
Copy link
Contributor Author

@eradman

Thank your for your review !!

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