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: Ensure no double scrollbars for NcDialog and NcAppNavigationSettings #5139

Merged
merged 3 commits into from Jan 26, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jan 25, 2024

☑️ Resolves

  • Fix [BITV]: settings modal has double scrollbar under some circumstances nextcloud/server#42938

  • NcDialog:

    • Adjust styles to prevent two scrollbars on small screens
      This also fixes some design issues, we do not want padding or margin on the bottom and right of the content in case of a scrollbar. When scrolling the scrollbar should be on the very left (not floating in the middle) and the scrolled content should be clipped by the modal container.
    • Also ensure the dialog name is not overflown by the close button.
  • NcModal:

    • Use more height for a modal when window height is small, not only when window width is small.
  • NcAppNavigationSettings

    • Apply padding also for dialog name on small sizes where the name is not centered

🖼️ Screenshots

Note the double scrollbars

🏚️ Before 🏡 After
Screen Shot 2024-01-25 at 19 06 10 Screen Shot 2024-01-25 at 19 06 13
Screen Shot 2024-01-25 at 19 04 24 Screen Shot 2024-01-25 at 19 05 23

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@susnux susnux added bug Something isn't working 3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Jan 25, 2024
@susnux susnux added this to the 8.5.1 milestone Jan 25, 2024
src/components/NcDialog/NcDialog.vue Outdated Show resolved Hide resolved
susnux and others added 3 commits January 26, 2024 20:18
This also fixes some design issues, we do not want padding or margin on the bottom and right of the content in case of a scrollbar.
When scrolling the scrollbar should be on the very left (not floating in the middle) and the scrolled content should be clipped by the modal container.

Also ensure the dialog name is not overflown by the close button.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…uble scrollbars

Co-authored-by: Ferdinand Thiessen <opensource@fthiessen.de>
Co-authored-by: Eduardo Morales <emoral435@gmail.com>
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux enabled auto-merge January 26, 2024 19:19
@susnux susnux merged commit 0e5baf5 into master Jan 26, 2024
15 checks passed
@susnux susnux deleted the fix/nc-dialog branch January 26, 2024 19:21
@susnux susnux mentioned this pull request Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BITV]: settings modal has double scrollbar under some circumstances
5 participants