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(protocol-designer): fix scroll position of MoreOptionsModal #6515

Merged
merged 1 commit into from
Sep 14, 2020

Conversation

IanLondon
Copy link
Contributor

@IanLondon IanLondon commented Sep 10, 2020

Overview

Closes #6156

Changelog

  • refactor all places I could find in PD where we do element.scrollToTop = 0 to use a common util
  • fix scroll of MoreOptionsModal

Review requests

I found there were 2 ways of doing the same thing for the scroll to top behavior, and a lot of repeated code for it, so I factored it out to a new resetScrollElements util. The comment about the old issue #4446 still stands, but IMO probably not a priority to address that until we do larger styling changes to PD one day. (Agree/disagree?)

Since I did that refactor, we should double-check the scroll behavior of several elements. To check, scroll down as far as you can while still keeping the button you need in view, and ensure that the dark gray modal overlay fills the main panel (instead of weirdly only partially filling it)

  • The one the issue bug: PD "Notes" modal can appear off-screen and you can't scroll to it #6156 is actually for: MoreOptionsModal (click "NOTES" button in a step form)
  • BrowseLabware modal (open a step form so you can scroll down, and click "View Liquids" on a labware on the deck)
  • edit pipette modal
  • add/edit module modal
  • file export warning modals
  • experimental settings warning modal
  • selecting a step should still scroll to top (via select step thunk)

Risk assessment

Low, at worst could mess up scroll behaviors of other modals

@IanLondon IanLondon added ready for review protocol designer Affects the `protocol-designer` project labels Sep 10, 2020
@IanLondon IanLondon requested review from Kadee80, shlokamin and a team September 10, 2020 14:29
@IanLondon IanLondon self-assigned this Sep 10, 2020
Copy link
Member

@shlokamin shlokamin left a comment

Choose a reason for hiding this comment

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

thanks for getting rid of the repeated code!

@IanLondon IanLondon merged commit 2bf776b into edge Sep 14, 2020
@IanLondon IanLondon deleted the pd_fix-notes-modal-scroll branch September 14, 2020 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
protocol designer Affects the `protocol-designer` project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: PD "Notes" modal can appear off-screen and you can't scroll to it
2 participants