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

Stop add-on Draggable from overlapping the vertical scrollbar when stories overflow #17663

Merged

Conversation

LucaCras
Copy link
Contributor

@LucaCras LucaCras commented Mar 8, 2022

Issue: #17202

fix the draggable overlaying the vertical scrollbar when stories overflow at the bottom, making the story un-scrollable.

What I did

There was a negative margin on the Draggable component that is responsible for resizing the add-on panel when in side-panel mode.
This margin pushed the Draggable to the left by 10px, which caused it to overlap with the vertical scrollbar for scrolling down a long story.

I have removed this margin, which puts the Draggable in the correct spot.

Below is an image illustrating the old- (yellow) & new (blue) positions.

Group

Schermopname.2022-03-16.om.16.16.38.mov

How to test

  1. Create a story that overflows on the bottom.
  2. View the story.
  3. Try to scroll down with the right-hand scrollbar. (should be able to use the entire width of the scrollbar)
  4. Try to resize the add-on panel in side-panel mode. (should be at the correct position, not overlapping anything)
  • Is this testable with Jest or Chromatic screenshots?
  • Does this need a new example in the kitchen sink apps?
  • Does this need an update to the documentation?

If your answer is yes to any of these, please make sure to include it in your PR.

…flow at the bottom, making the story unscrollable
@nx-cloud
Copy link

nx-cloud bot commented Mar 8, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit e962b47. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@yannbf
Copy link
Member

yannbf commented Mar 14, 2022

hey @LucaCras thanks a lot for your contribution! There's a minor detail (which we discussed in triage) so let us know if you can revisit that and if you ever want assistance feel free to ping!

@MichaelArestad
Copy link
Contributor

I am happy to help with anything design-related. I could do some ideation and propose a few things if you prefer.

@LucaCras
Copy link
Contributor Author

Hi @MichaelArestad, I have made a change that fixes the shadow to show up on the left side, instead of right, I have uploaded a video in the original message. Can you check if this works? I have also added a 1px margin, so there is no overlap between the iframe and the panel / draggable handle

Copy link
Contributor

@MichaelArestad MichaelArestad left a comment

Choose a reason for hiding this comment

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

@LucaCras This is quite nice. Nice work!

@LucaCras
Copy link
Contributor Author

@LucaCras This is quite nice. Nice work!

Thanks! Is there anything else I need to do @MichaelArestad?

@shilman shilman assigned ndelangen and unassigned MichaelArestad Mar 28, 2022
@MichaelArestad
Copy link
Contributor

@LucaCras I don't think so. This is just awaiting 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.

5 participants