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 omnibar scrolling and browser layout gap #5131

Merged
merged 5 commits into from
Oct 14, 2024

Conversation

0nko
Copy link
Member

@0nko 0nko commented Oct 12, 2024

Task/Issue URL: https://app.asana.com/0/488551667048375/1208503210252671/f
Also: https://app.asana.com/0/488551667048375/1208503210253722/f.

Description

This PR fixes 2 connected issues:

  • A gap at the bottom of a screen is visible when scroll flags are set on a toolbar container to disable toolbar scrolling
  • Bottom omnibar not visible or is incorrectly scrollable while an omnibar icon is highlighted

The solution unifies how the scrolling is enabled/disabled for both the top and bottom bar. The scroll flags are not set anymore, which fixes the gap problem.

Steps to test this PR

Bottom bar scrolling with highlighted icons

  • Clear the app storage and run the app
  • Set the omnibar position to bottom
  • Go through the onboarding steps until you get to the blocked trackers
  • Notice that when the shield is highlighted, the omnibar is expanded and not scrollable
  • After completing the onboarding, notice that the omnibar is scrollable again

Extra gap at the bottom

  • Clear the app storage and run the app
  • Go through the onboarding steps until you get to the blocked trackers
  • Notice there is no gap at the bottom

UI changes

Bottom bar scrolling with highlighted icons

Screen_recording_20241012_165918.mp4

Extra gap at the bottom

Screen_recording_20241012_165828.mp4

@0nko 0nko requested a review from nalcalag October 12, 2024 15:00
@nalcalag nalcalag self-assigned this Oct 14, 2024
Copy link
Contributor

@nalcalag nalcalag left a comment

Choose a reason for hiding this comment

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

@0nko I've been testing this and although the bugs seem to be fixed 🎉 . There is a padding added to the top of the bottom address bar that is visible when a Dax dialog is shown in the New Tab Page. See video attached.

Screen_recording_20241014_092705.mp4

@0nko
Copy link
Member Author

0nko commented Oct 14, 2024

Thanks for the review @nalcalag! I added a change that fixes the issue. Ready for another round!

@0nko 0nko requested a review from nalcalag October 14, 2024 10:44
Copy link
Contributor

@nalcalag nalcalag left a comment

Choose a reason for hiding this comment

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

LGTM! All bugs are fixed 💯

@0nko 0nko enabled auto-merge (squash) October 14, 2024 11:14
@0nko 0nko merged commit 54d2d1a into develop Oct 14, 2024
5 checks passed
@0nko 0nko deleted the fix/ondrej/omnibar-gap-and-scrolling branch October 14, 2024 11:28
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