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

Sign up
Appearance settings

horizontal legend width should be layout width #5108

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

Open
p1-ra wants to merge 1 commit into plotly:master
base: master
Choose a base branch
Loading
from p1-ra:fix/horizontal-legend-container-width

Conversation

Copy link

@p1-ra p1-ra commented Aug 29, 2020

Currently, horizontal legend width is set to the width of its longer text. It make the display of the scrollbar a little bit disturbing,

Capture_hlegend_broken

This PR fix horizontal legend width and set it to the full layout width:

Capture_hlegend_fixed

Copy link
Contributor

archmoj commented Sep 1, 2020

Thanks for the PR.
Seems like a good catch at first glance.
However there are a number of baselines that change.
So maybe one could consider enabling this option as a feature.

For now, could you please regenerate these?

geo_choropleth-legend
gl2d_fill-ordering
gl2d_parcoords_blocks
gl3d_mesh3d_coloring
legend_horizontal
legend_horizontal_wrap-alll-lines
legend_negative_x
legend_negative_x2
legend_scroll_beyond_plotarea
legendgroup_horizontal_bg_fit
pie_automargin
uniformtext_bar-like_10_auto
uniformtext_bar-like_8_textangle
uniformtext_bar-like_8_textangle45

Also please visit https://github.com/plotly/plotly.js/blob/master/CONTRIBUTING.md#image-pixel-comparison-tests for more info regarding generating the baselines.

@@ -742,6 +742,9 @@ function computeLegendDimensions(gd, groups, traces, opts) {
opts._titleWidth + 2 * (bw + constants.titlePad)
)
);
if (!isVertical) {
Copy link
Contributor

@archmoj archmoj Sep 1, 2020

Choose a reason for hiding this comment

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

To pass the syntax test,
no space required after if statement.

if(!isVertical) {

Also you could run syntax test locally using:

npm run test-syntax

Copy link
Collaborator

Interesting, I certainly see the appeal of this, but I feel like it should at least be scoped to legends that have a scrollbar (the difference becomes important if the legend has a background or border), and possibly (as @archmoj mentioned) be turned into an opt-in attribute like legend.width

archmoj reacted with thumbs up emoji

@archmoj archmoj added status: in progress community community contribution feature something new labels Sep 23, 2020
Copy link
Contributor

A small update on timing: our team is working hard on releasing v2.0 of Plotly.js, which we anticipate will happen in early April. This PR would be a good candidate to land in the library in v2.1 or later, so with apologies for the delay, we will likely not be able to give much feedback on this PR for the next few weeks :)

@gvwilson gvwilson self-assigned this Jun 10, 2024
Copy link
Contributor

This pull request has been sitting for a while, so I would like to close it as part of our effort to tidy up our public repositories. I've assigned it to myself to keep track of it; I'll wait until 2024年06月17日 for someone to say it's still relevant and they'll to take it on, and otherwise I will close it then. Thanks - @gvwilson

@gvwilson gvwilson removed their assignment Aug 2, 2024
@gvwilson gvwilson changed the title (削除) Fix: horizontal legend width should be layout width (削除ここまで) (追記) horizontal legend width should be layout width (追記ここまで) Aug 8, 2024
@gvwilson gvwilson added fix fixes something broken P2 considered for next cycle and removed feature something new labels Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marthacryan marthacryan Awaiting requested review from marthacryan

1 more reviewer

@archmoj archmoj archmoj left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
community community contribution fix fixes something broken P2 considered for next cycle
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /