Omnibar position: UI fixes #5003

0nko · 2024-09-12T10:44:12Z

Task/Issue URL: https://app.asana.com/0/1207418217763355/1208269999932720/f

Description

This PR contains multiple design fixes but mainly addresses an issue with a fixes-size websites that were covered by the bottom toolbar.

Fixes issues:

Browser menu not shown at the correct position after tapping the menu button with the keyboard open
Incorrect autocomplete suggestion icons
Bottom toolbar covering up the web content of fixed-sized websites
Tapping on a bottom toolbar’s edges leaked clicks to the web view
Onboarding dialog and NTP had extra padding at the top when toolbar at the bootom
There was a zombie shadow of the toolbar

UI changes

Screen_recording_20240913_015829.mp4

0nko · 2024-09-12T10:44:17Z

Omnibar Refactor: Feature Flag #5035
Omnibar position: UI fixes #5003 👈
Omnibar position: Translations #4993
Omnibar position: Pixels #4974
Omnibar position option #4885 : 4 other dependent PRs (#4886 , #4940 , #4941 and 1 other)
develop

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @0nko and the rest of your teammates on Graphite

malmstein

Added some early feedback. Overall the solution looks much better than before, but I’ll hold any more review until the outstanding issues are solved.

app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt

malmstein · 2024-09-16T08:20:13Z

app/src/main/java/com/duckduckgo/app/browser/omnibar/BottomAppBarBehavior.kt

@@ -76,6 +76,7 @@ class BottomAppBarBehavior<V : View>(context: Context, attrs: AttributeSet) : Co
    ) {
        super.onNestedPreScroll(coordinatorLayout, child, target, dx, dy, consumed, type)

+        // don't hide the app bar in the autocomplete suggestions list or the NTP
        if (target.id != R.id.autoCompleteSuggestionsList && target.id != com.duckduckgo.newtabpage.impl.R.id.newTabContentScroll) {


is targetting specific ids safe? Can we pass the ids as part of the class constructor instead?

The behavior is applied in the layout, as well, so probably not. But I’ve since changed this because the only target we really want is the webView. See the latest changes.

…ection

Task/Issue URL: https://app.asana.com/0/0/1208157864474373/f ### Description ### Steps to test this PR - [ ] Start the omnibar at the top - [ ] Reload the page and notice the dummy toolbar is shown at the top - [ ] Change the omnibar to the bottom - [ ] Reload the page and notice the dummy toolbar is shown at the bottom

Task/Issue URL: https://app.asana.com/0/1207418217763355/1208166986247266/f - [ ] Change the omnibar position to the bottom - [ ] Start typing in the address bar - [ ] Notice the suggestions are correctly displayed above the toolbar | Before | After | | ------ | ----- | ![image](https://github.com/user-attachments/assets/dfae5e47-a6e1-4d4d-bc25-4847ef4a0a32)|![image](https://github.com/user-attachments/assets/7ef7a0f9-cc74-41f5-8f28-d1d3462b740f)|

Translate strings to values-es Translate strings to values-ro Translate strings to values-tr Translate strings to values-sk Translate strings to values-ru Translate strings to values-et Translate strings to values-it Translate strings to values-pl Translate strings to values-bg Translate strings to values-lv Translate strings to values-fi Translate strings to values-sl Translate strings to values-nl Translate strings to values-hr Translate strings to values-hu Translate strings to values-nb Translate strings to values-sv Translate strings to values-da Translate strings to values-el Translate strings to values-fr Translate strings to values-lt Translate strings to values-pt Translate strings to values-cs

This reverts commit 353fdf6.

…le it so snackbar offsetting works

…omnibar

…t.kt Co-authored-by: David González <malmstein@gmail.com>

…kduckgo/Android into feature/ondrej/omnibar-postion-fixes

Task/Issue URL: https://app.asana.com/0/1207418217763355/1208269999932720/f This PR contains multiple design fixes but mainly addresses an issue with a fixes-size websites that were covered by the bottom toolbar. Fixes issues: - Browser menu not shown at the correct position after tapping the menu button with the keyboard open - Incorrect autocomplete suggestion icons - Bottom toolbar covering up the web content of fixed-sized websites - Tapping on a bottom toolbar’s edges leaked clicks to the web view - Onboarding dialog and NTP had extra padding at the top when toolbar at the bootom - There was a zombie shadow of the toolbar https://github.com/user-attachments/assets/c5a7ae9a-f543-433b-9ef7-562f17f3af39 ![Screenshot_20240910_132343](https://github.com/user-attachments/assets/d7c864aa-0912-4ca7-b046-aa14b97a8202) ![Screenshot_20240910_132443](https://github.com/user-attachments/assets/48a78d4a-5860-4e00-a0dc-922aae7d3ef5) ![Screenshot_20240913_003744](https://github.com/user-attachments/assets/3cffb693-7eac-413f-a304-436e6b3795fa) ![Screenshot_20240913_003843](https://github.com/user-attachments/assets/63b1f5a9-fd03-4cbd-a089-5c6dae18ae04) ![Screenshot_20240913_003932](https://github.com/user-attachments/assets/8e6f9172-1833-4899-a544-1ec922926740) ![Screenshot_20240913_004024](https://github.com/user-attachments/assets/054fea28-267b-4d7f-a91b-c4e4df26d021) ![Screenshot_20240913_011751](https://github.com/user-attachments/assets/28a5ed01-91b7-480d-80cc-644d6a4e94f1) --------- Co-authored-by: Dax The Translator <daxmobile@duckduckgo.com> Co-authored-by: David González <malmstein@gmail.com>

This was referenced Sep 12, 2024

Omnibar position: Pixels #4974

Merged

Omnibar position: Translations #4993

Merged

0nko force-pushed the feature/ondrej/omnibar-position-translation branch from 6b11d0b to 39c7a9f Compare September 12, 2024 11:58

0nko force-pushed the feature/ondrej/omnibar-postion-fixes branch 2 times, most recently from 4e42bc7 to 30c23f5 Compare September 13, 2024 07:42

0nko marked this pull request as ready for review September 13, 2024 12:21

0nko requested a review from malmstein September 13, 2024 12:21

malmstein self-assigned this Sep 13, 2024

malmstein reviewed Sep 16, 2024

View reviewed changes

0nko force-pushed the feature/ondrej/omnibar-postion-fixes branch from d4df09f to 0bb3b70 Compare September 17, 2024 14:32

0nko requested a review from CDRussell as a code owner September 17, 2024 14:32

0nko force-pushed the feature/ondrej/omnibar-position-translation branch from 62f9087 to 651f9a4 Compare September 17, 2024 14:33

0nko and others added 17 commits September 19, 2024 10:37

Track the omnibar position selection in the settings

b737423

Add a new plugin type that can be used to report browser parameters

d5670db

Add a plugin that detects the current omnibar position for stats coll…

cf89b00

…ection

Add settings unit tests for omnibar position

8950c01

Fix ktlint issues

9077f7b

Optimize imports

937d780

Autoformat kotlin

b21f3bd

Fix ktlint issues

a9c3d4f

Update the position detector plugin to use the new API

589e086

Move new strings for translation

e06223e

Revert "Translate strings to values-de"

d29ee92

This reverts commit 353fdf6.

Remove old strings

4a88c2c

Spotless fix

f6557ca

0nko added 15 commits September 19, 2024 11:00

Fix the omnibar icon paddings and make them match with the mock

cb11310

Add a padding to the snackbar

3dd929e

Always show an omnibar after a new page is loaded

a3a27ec

Fix the build

24781f8

Fix the bottom toolbar hiding during non-browser background scrolling

39879c7

Improve the blocked scrolling javascript code

0a66b11

Fix the scrolling detection JS code

191894d

Instead of removing the behavior for non-scrollable sites, just disab…

7e8eb74

…le it so snackbar offsetting works

Replace the included bottom omnibar layout with a view layout

5d1c10b

Refactor the LegacyOmnibarView so that it can be used for the bottom …

2e1a03b

…omnibar

Refactor the Omnibar to use the new LegacyOmnibarView for both positions

6c23f3e

Fix ktlint issues

943002f

Fix the bottom bar behavior

0410c46

Add a feature flag check for the omnibar position

036cc41

Add a feature flag to the omnibar position settings

5489ecb

0nko force-pushed the feature/ondrej/omnibar-position-translation branch from 651f9a4 to f6557ca Compare September 19, 2024 09:00

0nko mentioned this pull request Sep 19, 2024

Omnibar Refactor: Feature Flag #5035

Closed

0nko force-pushed the feature/ondrej/omnibar-postion-fixes branch from 81f856a to 5489ecb Compare September 19, 2024 09:00

0nko and others added 5 commits September 19, 2024 11:02

Update app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragmen…

7eb1b16

…t.kt Co-authored-by: David González <malmstein@gmail.com>

Update the function name

09e341c

Merge branch 'feature/ondrej/omnibar-postion-fixes' of github.com:duc…

9bf27a5

…kduckgo/Android into feature/ondrej/omnibar-postion-fixes

Fix ktlint issues

180431d

Disable the omnibar position feature flag by default

e68045d

malmstein approved these changes Sep 19, 2024

View reviewed changes

Base automatically changed from feature/ondrej/omnibar-position-translation to feature/ondrej/omnibar-position September 19, 2024 10:33

0nko merged commit 04e15a0 into feature/ondrej/omnibar-position Sep 19, 2024
5 of 6 checks passed

0nko deleted the feature/ondrej/omnibar-postion-fixes branch September 19, 2024 10:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Omnibar position: UI fixes #5003

Omnibar position: UI fixes #5003

0nko commented Sep 12, 2024 •

edited

Loading

0nko commented Sep 12, 2024 •

edited

Loading

malmstein left a comment

malmstein Sep 16, 2024

0nko Sep 19, 2024

Omnibar position: UI fixes #5003

Omnibar position: UI fixes #5003

Conversation

0nko commented Sep 12, 2024 • edited Loading

Description

UI changes

0nko commented Sep 12, 2024 • edited Loading

malmstein left a comment

Choose a reason for hiding this comment

malmstein Sep 16, 2024

Choose a reason for hiding this comment

0nko Sep 19, 2024

Choose a reason for hiding this comment

0nko commented Sep 12, 2024 •

edited

Loading

0nko commented Sep 12, 2024 •

edited

Loading