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: Preserve dimensions on resizing for panel #6696

Merged
merged 1 commit into from
May 6, 2019

Conversation

kohakukun
Copy link
Contributor

Issue: #6343

What I did

  • Update resizer position when changing the boundaries of the container
    for the addon panel and preview

How to test

storybookPR

- Update resizer position when changing the boundaries of the container
for the addon panel and preview
@vercel
Copy link

vercel bot commented May 1, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://monorepo-git-fork-kohakukun-fix-resizingerror.storybook.now.sh

@kohakukun
Copy link
Contributor Author

@shilman PTAL.

P.S.: Not sure what to do about the eslint rule. Should we use another solution?

@shilman shilman added bug patch:yes Bugfix & documentation PR that need to be picked to main branch ui labels May 1, 2019
@shilman shilman added this to the 5.0.x milestone May 1, 2019
const { width: prevWidth, height: prevHeight } = prevProps.bounds;
const { bounds, options } = this.props;
const { width, height } = bounds;
if (width !== prevWidth || height !== prevHeight) {
Copy link
Member

Choose a reason for hiding this comment

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

Why can't this block be done in the getDerivedStateFromProps ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to move the resizer by the delta of boundaries. I could move it to getDeriverStateFromProps by changing the resizerPanel state to store width and height instead of a position. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

yeah, It may work I think.

@ndelangen ndelangen self-assigned this May 6, 2019
@ndelangen
Copy link
Member

Thank you @kohakukun !

@Aaron-Pool
Copy link
Contributor

@kohakukun OH MY GOSH THANK YOU. This was driving me crazy.

@shilman shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug patch:done Patch/release PRs already cherry-picked to main/release branch patch:yes Bugfix & documentation PR that need to be picked to main branch ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants