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

Fix addon panel layout styling #1170

Merged
merged 2 commits into from
Jun 2, 2017
Merged

Fix addon panel layout styling #1170

merged 2 commits into from
Jun 2, 2017

Conversation

zillding
Copy link
Contributor

@zillding zillding commented May 31, 2017

Issue: #1169

Steps to reproduce:

  1. run the storybook dev server
  2. point to ui localhost:9001 using safari
  3. press cmd + shift + j on mac to change the add on panel from bottom to right side
  4. the right panel will look like the screenshot in Addon panel is not positioned properly in vertical mode in safari #1169

What I did

Add an additional css rule position: absolute to make sure the panel fill the entire space.

How to test

Tested manually in safari, firefox and chrome.

@@ -22,6 +22,7 @@ const downPanelStyle = downPanelInRight => ({
display: 'flex',
flexDirection: downPanelInRight ? 'row' : 'column',
alignItems: 'stretch',
position: 'absolute',
Copy link
Member

Choose a reason for hiding this comment

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

This will break scrolling inside the panel, I think?

I'm confused about this element being width & height : 100% AND also have padding..

Can you provide a reproducible case which caused you to find a problem you wanted to fix?

Copy link
Member

Choose a reason for hiding this comment

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

Whoops I missed the issue, I'm going have to try opening safari I guess 😃

@ndelangen
Copy link
Member

Thanks for the PR! Can you give me a bit more background on why that was needed and can you provide a full steps to reproduce, so I can consider if this is the best solution?

😄

@codecov
Copy link

codecov bot commented May 31, 2017

Codecov Report

Merging #1170 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1170   +/-   ##
=======================================
  Coverage   13.31%   13.31%           
=======================================
  Files         199      199           
  Lines        4588     4588           
  Branches      724      724           
=======================================
  Hits          611      611           
  Misses       3349     3349           
  Partials      628      628
Impacted Files Coverage Δ
lib/ui/src/modules/ui/components/layout/index.js 73.58% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3868a54...56e596f. Read the comment docs.

@zillding
Copy link
Contributor Author

🙇 Thanks for the feedback.

Steps to reproduce:

  1. run the storybook dev server
  2. point to ui localhost:9001 using safari
  3. press cmd + shift + j on mac to change the add on panel from bottom to right side
  4. the right panel will look like the screenshot in Addon panel is not positioned properly in vertical mode in safari #1169

@zillding
Copy link
Contributor Author

I am not sure whether this is the best solution. This is the minimum change it seems to take. 😊

The scroll seems still working.

scroll

Change before and after:
css

@ndelangen
Copy link
Member

Thank you for the attention to detail in your explanation, it helps!

I'll rebase and merge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants