Skip to content

fix volcano margins for legend#551

Closed
akmorrow13 wants to merge 2 commits into
plotly:masterfrom
akmorrow13:volcano_margins
Closed

fix volcano margins for legend#551
akmorrow13 wants to merge 2 commits into
plotly:masterfrom
akmorrow13:volcano_margins

Conversation

@akmorrow13

@akmorrow13 akmorrow13 commented Apr 3, 2021

Copy link
Copy Markdown

Closes #[issue number; if this PR doesn't address an issue, delete this line]

About

  • I am closing an issue
  • This is a new component
  • I am adding a feature to an existing component, or improving an existing feature

Description of changes

@akmorrow13

akmorrow13 commented Apr 3, 2021

Copy link
Copy Markdown
Author

Adjusted the margins so the plot can take up more space.

BEFORE:
Screen Shot 2021-04-03 at 9 58 51 AM

AFTER:
Screen Shot 2021-04-03 at 9 57 01 AM

@xhluca xhluca left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks so much better! 💃

@xhluca

xhluca commented Apr 6, 2021

Copy link
Copy Markdown

@alexcjohnson @HammadTheOne Any thoughts?

@HammadTheOne

Copy link
Copy Markdown
Contributor

@alexcjohnson @HammadTheOne Any thoughts?

While it does make the margins what we want them, I'm very hesitant to modify the underlying volcano_plot component arguments directly with hardcoded values in this PR. We could also just change the styling of the parent component using CSS selectors (eg. .control-tabs) to make it more readable.

I think these arguments should be allowed to be passed in by users anyways, so I can open a separate PR that enables that and sets the current ones as defaults. I'll link it to this PR once it's opened.

@HammadTheOne HammadTheOne mentioned this pull request Apr 7, 2021
1 task
@HammadTheOne

Copy link
Copy Markdown
Contributor

PR for VolcanoPlot updates is opened here: #554

Once that's merged, we'll be able to pass in arguments for the Layout of the figure to set margin, height, width, or any other key referenced here.

@akmorrow13

Copy link
Copy Markdown
Author

Closing in favor of #554

@akmorrow13 akmorrow13 closed this Apr 11, 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

Development

Successfully merging this pull request may close these issues.

3 participants