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

Remove code that stops settings from scrolling on shorter screens #1767

Merged
merged 2 commits into from
Dec 10, 2019

Conversation

belcherj
Copy link
Contributor

@belcherj belcherj commented Dec 9, 2019

Fix

At some point, some code was added that allowed the tabs to scroll. In adding
this it caused the settings tab to display at full height and not be contained
by the scroll box in the modal. This fixes
https://href.li/?https://github.com/Automattic/simplenote-electron/issues/1268

This is not a long term solution as it is still not a great user experience. We need to drop the modals for full-page settings.

Test

  1. Open app.simplenote.com with dev tools open
  2. Open settings to the display tab
  3. Enable mobile responsive mode
  4. Set the height to less than 640px
  5. See that it doesn't scroll
  6. Do the same on this branch
  7. Observe that it does scroll

Old, doesn't scroll:

old

After, scrolls:

Dec-09-2019 14-28-09

Review

Only one developer is required to review these changes, but anyone can perform the review.

Release

RELEASE-NOTES.txt was updated with:

  • Makes settings scrollable on shorter smaller view ports #1767

At some point, some code was added that allowed the tabs to scroll. In adding
this it caused the settings tab to display at full height and not be contained
by the scroll box in the modal. This fixes
https://href.li/?https://github.com/Automattic/simplenote-electron/issues/1268
@belcherj belcherj requested a review from a team December 9, 2019 19:25
@belcherj belcherj self-assigned this Dec 9, 2019
Copy link
Member

@codebykat codebykat left a comment

Choose a reason for hiding this comment

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

Looks reasonable, fixes the issue 👍

@belcherj belcherj merged commit 8c5227f into develop Dec 10, 2019
@belcherj belcherj deleted the fix/settigns-doesnt-scroll branch December 10, 2019 15:00
@codebykat codebykat added this to the 1.14 milestone Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants