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(radio): modal jumping when clicking radio #231

Merged
merged 1 commit into from
Aug 14, 2020

Conversation

Birkbjo
Copy link
Member

@Birkbjo Birkbjo commented Aug 7, 2020

See Jira issue with link to codesandbox: https://jira.dhis2.org/browse/DHIS2-9104

When clicking a radio in a modal that is scrollable, the modal will scroll and the content will be outside of the viewport. This is due to how the modal is scrolling the modal-content only, using display: flex. I believe the reason for using flex here is to keep the header above the content when scrolling.

The Card rendered by the Modal will have a height of something like 2x of the modal-content. So when a Radio is clicked, the page is for some weird reason (see https://stackoverflow.com/questions/24299567/radio-button-causes-browser-to-jump-to-the-top/24323870) scrolled to the position of the clicked Radio in the Card-div. As far as I understand this, it's due to the position: absolute on the Radios, and the Card being the closest relative-positioned element.

This fix makes the label positioned relative, so when a Radio is clicked, it will not scroll to the position. However, this could potentially make absolutely position elements in the modal behave weirdly, so another solution would be to have a simple display: block on the Card ( instead of flex) - but that would mean we would scroll the header as well.
Maybe the ModalContent should have position: relative as well, to play nice with absolutely positioned elements?

@Mohammer5 replied that position relative is not ideal. The inputs are already hidden with opacity: 0, and it looks like visibility: hidden works as well. Source for fix in above stackoverflow.

@Birkbjo Birkbjo requested a review from a team as a code owner August 7, 2020 17:22
@cypress
Copy link

cypress bot commented Aug 7, 2020



Test summary

484 0 0 0


Run details

Project ui
Status Passed
Commit c8a2671
Started Aug 14, 2020 10:07 AM
Ended Aug 14, 2020 10:18 AM
Duration 11:06 💡
OS Linux Ubuntu Linux - 18.04
Browser Electron 80

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@Birkbjo Birkbjo changed the title fix(radio): position label relative fix(radio): modal jumping when clicking radio Aug 11, 2020
@Birkbjo Birkbjo force-pushed the dhis2-9104/fix-modal-jump-when-element-select branch from ffd9cb6 to 9d3fc96 Compare August 11, 2020 09:22
@Birkbjo
Copy link
Member Author

Birkbjo commented Aug 14, 2020

We are using the input to handle focus, so visibility: hidden makes it no-focusable. So I would want to go back to have position: relative on the label.

@Birkbjo Birkbjo force-pushed the dhis2-9104/fix-modal-jump-when-element-select branch from e70e724 to c8a2671 Compare August 14, 2020 10:03
@Birkbjo Birkbjo merged commit 82a34c1 into master Aug 14, 2020
@Birkbjo Birkbjo deleted the dhis2-9104/fix-modal-jump-when-element-select branch August 14, 2020 10:38
@dhis2-bot
Copy link
Contributor

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.

3 participants