Skip to content

Split Omnibar: Address bar integration #7042

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 5, 2025
Merged

Split Omnibar: Address bar integration #7042

merged 4 commits into from
Nov 5, 2025

Conversation

@0nko
Copy link
Member

@0nko 0nko commented Oct 31, 2025 •
edited
Loading

Task/Issue URL: https://app.asana.com/1/137249556945/project/1207418217763355/task/1211565479656355?focus=true

Description

This PR adds the last integration of the split omnibar to the address bar:

  • When the split omnibar type is selected, the top omnibar buttons are hidden
  • The mock toolbar must show the split omnibar when loading

Steps to test this PR

Omnibar button

  • Enable the splitOmnibar flag
  • Go to Settings -> Appearance
  • Select the split omnibar
  • Go back to the browser
  • Notice the buttons next to the address bar are gone

Mock split toolbar

  • Restart the app
  • While the app is loading, notice the mock top bar and bottom navigation bar are displayed until the app's loaded

This was referenced Oct 31, 2025
Merged
Merged
Merged
@0nko Graphite App
Copy link
Member Author

0nko commented Oct 31, 2025 •
edited
Loading

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

@0nko 0nko mentioned this pull request Oct 31, 2025
Merged
20 tasks
@0nko 0nko changed the title Hide the icon container for split omnibar Split Omnibar: Address bar integration Oct 31, 2025
@0nko 0nko marked this pull request as ready for review October 31, 2025 21:09
@0nko 0nko requested a review from LukasPaczos October 31, 2025 21:09
@0nko 0nko force-pushed the feature/ondrej/split-omnibar-bottom-navigation branch from 09fe074 to b658d9f Compare November 3, 2025 10:38
@0nko 0nko force-pushed the feature/ondrej/split-omnibar-address-bar branch from 6a5cb77 to 539390e Compare November 3, 2025 10:38
@0nko 0nko mentioned this pull request Nov 3, 2025
Merged
12 tasks
@0nko 0nko force-pushed the feature/ondrej/split-omnibar-bottom-navigation branch from 1837d54 to 34b46fd Compare November 3, 2025 18:10
@0nko 0nko force-pushed the feature/ondrej/split-omnibar-address-bar branch from 539390e to 697f6fb Compare November 3, 2025 18:10
Base automatically changed from feature/ondrej/split-omnibar-bottom-navigation to develop November 3, 2025 18:34
@0nko 0nko force-pushed the feature/ondrej/split-omnibar-address-bar branch from 065ac0e to 7b19df7 Compare November 3, 2025 18:36
@0nko 0nko requested review from malmstein and removed request for LukasPaczos November 3, 2025 18:44
malmstein
malmstein requested changes Nov 3, 2025
View reviewed changes
Copy link
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

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

Let’s avoid creating more flags that duplicate what’s already existing.

app/src/main/java/com/duckduckgo/app/browser/omnibar/OmnibarLayoutViewModel.kt Outdated
private val _viewState = MutableStateFlow(
ViewState(
showChatMenu = duckAiFeatureState.showOmnibarShortcutInAllStates.value,
showButtons = settingsDataStore.omnibarType != OmnibarType.SPLIT,
Copy link
Contributor

@malmstein malmstein Nov 3, 2025

Choose a reason for hiding this comment

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

there is already a showTabsMenu, showFireIcon and showBrowserMenu, why not use those instead? showButtons is not really clear what it means

Copy link
Contributor

@malmstein malmstein Nov 3, 2025

Choose a reason for hiding this comment

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

I’d use the existing flags to decide if the container needs to be shown or not. Bonus points if doing that also prevents the Transition in renderButtons to run if the buttons are not visible.

app/src/main/java/com/duckduckgo/app/pixels/remoteconfig/AndroidBrowserConfigFeature.kt Outdated
* If the remote feature is not present defaults to `false`
*/
@Toggle.DefaultValue(DefaultFeatureValue.FALSE)
@Toggle.DefaultValue(DefaultFeatureValue.INTERNAL)
Copy link
Contributor

@malmstein malmstein Nov 3, 2025

Choose a reason for hiding this comment

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

this should be false by default, otherwise you’re turning this on for all internal users

app/src/main/java/com/duckduckgo/app/browser/BrowserTabFragment.kt Outdated
binding = binding,
isUnifiedOmnibarEnabled = omnibarFeatureRepository.isUnifiedOmnibarFlagEnabled,
)
configureNavigationBar()
Copy link
Contributor

@malmstein malmstein Nov 3, 2025

Choose a reason for hiding this comment

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

what’s the reason for this change?

Copy link
Member Author

@0nko 0nko Nov 4, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

To initialize the navigation bar earlier and reduce the flickering when loading. It helped a bit but did not fully work. I found the actual reason, so reverting (not that it really matters).

@0nko 0nko requested a review from malmstein November 4, 2025 07:39
@0nko
Copy link
Member Author

0nko commented Nov 4, 2025

@malmstein Thanks. Ready for another round.

@0nko 0nko force-pushed the feature/ondrej/split-omnibar-address-bar branch from e25a604 to 643feee Compare November 4, 2025 08:07
@0nko 0nko force-pushed the feature/ondrej/split-omnibar-address-bar branch from 7ebd273 to b136ce0 Compare November 4, 2025 16:35
@0nko 0nko mentioned this pull request Nov 4, 2025
Open
15 tasks
@malmstein malmstein self-assigned this Nov 4, 2025
malmstein
malmstein approved these changes Nov 4, 2025
View reviewed changes
Copy link
Contributor

@malmstein malmstein left a comment

Choose a reason for hiding this comment

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

works great, thanks @0nko !

@0nko 0nko merged commit 0973926 into develop Nov 5, 2025
13 of 14 checks passed
@0nko 0nko deleted the feature/ondrej/split-omnibar-address-bar branch November 5, 2025 10:43
This was referenced Nov 5, 2025
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@malmstein malmstein malmstein approved these changes

Assignees

@malmstein malmstein

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@0nko @malmstein