Skip to content

android: Convert navigation bar to Compose UI - #46164

Merged
yezhizhen merged 2 commits into
servo:mainfrom
veyndan:compose-ui-nav-bar
Jul 3, 2026
Merged

android: Convert navigation bar to Compose UI#46164
yezhizhen merged 2 commits into
servo:mainfrom
veyndan:compose-ui-nav-bar

Conversation

@veyndan

@veyndan veyndan commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Unlike the toolbar on larger screens, the navigation bar couldn't be migrated piecemeal, since the Android View enforces an all or nothing approach.

Testing: There are no automated tests for Android.
Fixes: Part of #45715.

Signed-off-by: veyndan <veyndan@gmail.com>
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 1, 2026
@@ -0,0 +1,4 @@
<?xml version="1.0" encoding="utf-8"?>
<resources>
<item type="id" name="cancel_menu_item" />

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Creating IDs manually like this is usually a bit odd, but I did it because MainActivity still needs IDs to "dispatch actions". We probably don't need this "dispatch action" logic, and the code should probably be refactored to remove the usage of IDs. In order to keep this PR focused though, this is a stopgap which will be removed in a future PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

)
NavigationBarItem(
selected = false,
onClick = { dispatchAction(if (isRefreshingState.value) R.id.cancel_menu_item else R.id.refresh_menu_item) },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it possible to extract these common if else into selecting one out of two NavigationBarItem, instead of setting content&handler internally?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Much cleaner!

Signed-off-by: veyndan <veyndan@gmail.com>
)
NavigationBarItem(
selected = false,
onClick = { dispatchAction(if (isRefreshingState.value) R.id.cancel_menu_item else R.id.refresh_menu_item) },

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Much cleaner!

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jul 3, 2026
@yezhizhen
yezhizhen added this pull request to the merge queue Jul 3, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 3, 2026
@yezhizhen
yezhizhen removed this pull request from the merge queue due to a manual request Jul 3, 2026
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 3, 2026
@yezhizhen
yezhizhen added this pull request to the merge queue Jul 3, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 3, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 3, 2026
@servo-highfive servo-highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 3, 2026
@yezhizhen
yezhizhen added this pull request to the merge queue Jul 3, 2026
@servo-highfive servo-highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-tests-failed The changes caused existing tests to fail. labels Jul 3, 2026
Merged via the queue into servo:main with commit b9eee4d Jul 3, 2026
40 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 3, 2026
@veyndan
veyndan deleted the compose-ui-nav-bar branch July 3, 2026 16:12
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.

3 participants