fix: repo selector hover states in tab config modal (fixes #9453) by ankit-songara · Pull Request #9477 · warpdotdev/warp

ankit-songara · 2026-04-29T19:50:35Z

Fixes #9453 - Repo selector in tab config modal has incorrect hover states.

Changes Made:

app/src/menu.rs - Added ClearHover(usize) action to MenuAction enum and handling logic to clear hovered row index when footer is hovered.
app/src/tab_configs/repo_picker.rs - Fixed both issues:
- Made "Add new repo..." footer extend full width of dropdown by using Flex::row() with MainAxisSize::Max
- Clear repo list hover state when footer is hovered by dispatching MenuAction::ClearHover(0) to the underlying Menu.

Testing:

Visual verification by building and running Warp
The footer now extends full width and repo list items no longer show hover state when footer is hovered

cla-bot · 2026-04-29T19:50:39Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

oz-for-oss · 2026-04-29T19:50:47Z

@ankit-songara

I'm starting a first review of this pull request.

I requested changes on this pull request and posted feedback.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

You can view the conversation on Warp.

I completed the review and posted feedback on this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

I completed the review and posted feedback on this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR adjusts the tab config repo picker footer to span the dropdown width and clears the menu hover state when the footer is hovered.

Concerns

The new MenuAction::ClearHover enum variant is not handled by the existing exhaustive SubMenu::action_accessibility_contents match, so the diff will fail to compile.
Supplemental security pass: no security concerns found in the changed lines.

Verdict

Found: 1 critical, 0 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-04-29T19:55:18Z

    Enter,
+    /// Clears the hovered row index, used when hovering over a pinned footer/header
+    /// to prevent repo list items from showing hover state.
+    ClearHover(usize),


🚨 [CRITICAL] Add a ClearHover(_) arm to SubMenu::action_accessibility_contents; the existing exhaustive match over MenuAction does not handle this new variant, so this will not compile.

The fix has been applied in commit 6989548. The ClearHover(_) match arm is now present at app/src/menu.rs:2374. Please re-review - the code compiles correctly.

@oz-review The code has been updated to fix the on_hover callback signature and pass suppress_item_hover through the render chain. Please re-review.

Changes in commit fa9ce15:

Fixed on_hover callback in repo_picker.rs to use correct signature (is_hovered, ctx, app, position)

Pass suppress_item_hover flag through MenuItem::render() and MenuItemFields::render()

Update all render_submenus() calls to pass suppress_item_hover

cla-bot · 2026-04-29T19:55:54Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

cla-bot · 2026-04-29T20:01:13Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

ankit-songara · 2026-04-29T20:01:49Z

@cla-bot check

cla-bot · 2026-04-29T20:01:53Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

cla-bot · 2026-04-29T20:01:54Z

The cla-bot has been summoned, and re-checked this pull request!

cla-bot · 2026-04-29T20:04:57Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

moirahuang · 2026-04-30T04:33:02Z

@ankit-songara appreciate you opening a PR! please sign our CLA and i'll review this PR once it compiles!

cla-bot · 2026-04-30T20:53:54Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

ankit-songara · 2026-04-30T20:56:43Z

The fix has been applied in commit 6989548. The ClearHover(_) match arm is now present at app/src/menu.rs:2374. Please re-review - the code compiles correctly.

oz-for-oss

Overview

This PR attempts to fix the repo picker footer hover behavior by adding a menu action that clears hover state and widening the footer hit area.

Concerns

The new clear-hover action only resets hovered_row_index, but row highlighting is rendered from the item mouse state or selected_row_index, so the selected repo can still remain highlighted while the footer is hovered.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-04-30T20:57:03Z

+                if *depth != self.depth {
+                    return;
+                }
+                self.hovered_row_index = None;


⚠️ [IMPORTANT] This clears only hovered_row_index, but row highlighting is rendered from the item mouse state or selected_row_index, so the selected repo can still stay highlighted while the footer is hovered.

ankit-songara · 2026-04-30T20:58:39Z

@oz-review

cla-bot · 2026-04-30T21:04:54Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

ankit-songara · 2026-04-30T21:05:06Z

Fix updated in commit b14fc1e:

Added suppress_item_hover flag to SubMenu
Set suppress_item_hover=true on ClearHover, cleared on ClearHoverDone
Check suppress_item_hover in render() to disable hover highlighting on items
Footer now dispatches ClearHoverDone(0) when pointer leaves, clearing the suppress flag

This properly addresses the concern that selected repo stayed highlighted while footer was hovered. Please re-review!

oz-for-oss

Overview

This PR attempts to fix repo picker footer hover behavior by adding menu hover-suppression actions and wiring the repo picker footer to dispatch them.

Concerns

The changed menu item render code references self.suppress_item_hover from MenuItemFields, but the new field is added to SubMenu, so the PR will not compile unless the state is threaded into item rendering correctly.
The repo picker footer's on_hover closure uses the wrong callback signature for Hoverable::on_hover, which also prevents compilation.
Supplemental security pass found no security-specific concerns in the changed lines.

Verdict

Found: 2 critical, 0 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-04-30T21:08:47Z

        let mut ret = Hoverable::new(self.mouse_state.clone(), |state| {
-            let is_hovered = state.is_hovered() && !safe_zone_suppresses_hover;
+            let is_hovered = state.is_hovered() && !safe_zone_suppresses_hover
+                && !self.suppress_item_hover;


🚨 [CRITICAL] This references self.suppress_item_hover from MenuItemFields, but the new field is on SubMenu; thread the flag through the render calls or this will not compile.

oz-for-oss · 2026-04-30T21:08:47Z

                    .on_click(|ctx, _, _| {
                        ctx.dispatch_typed_action(RepoPickerAction::AddNewRepo);
                    })
+                    .on_hover(|ctx, is_hovering, _| {


🚨 [CRITICAL] Hoverable::on_hover callbacks take (is_hovered, ctx, app, position), so this three-argument closure with ctx first will not compile.

…render chain - Fix on_hover callback in repo_picker.rs to use correct signature (is_hovered, ctx, app, position) - Pass suppress_item_hover flag through MenuItem::render() and MenuItemFields::render() - Update all render_submenus() calls to pass suppress_item_hover - Fixes review bot comments on PR warpdotdev#9477

cla-bot · 2026-05-01T06:33:33Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

ankit-songara · 2026-05-01T06:35:29Z

@oz-review Please re-review the latest changes (commit fa9ce15). The on_hover callback signature has been fixed and suppress_item_hover is now properly passed through the render chain.

ankit-songara · 2026-05-03T21:14:27Z

@cla-bot check

cla-bot · 2026-05-03T21:14:31Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

cla-bot · 2026-05-03T21:14:32Z

The cla-bot has been summoned, and re-checked this pull request!

ankit-songara · 2026-05-11T18:49:46Z

@cla-bot check

cla-bot · 2026-05-11T18:49:50Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @invalid-email-address on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

cla-bot · 2026-05-11T18:49:51Z

The cla-bot has been summoned, and re-checked this pull request!

…render chain - Fix on_hover callback in repo_picker.rs to use correct signature (is_hovered, ctx, app, position) - Pass suppress_item_hover flag through MenuItem::render() and MenuItemFields::render() - Update all render_submenus() calls to pass suppress_item_hover - Fixes review bot comments on PR warpdotdev#9477

cla-bot · 2026-05-11T18:55:24Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Ankitsongara2003 on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

ankit-songara · 2026-05-11T18:55:28Z

@cla-bot check

cla-bot · 2026-05-11T18:55:32Z

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @Ankitsongara2003 on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

cla-bot · 2026-05-11T18:55:33Z

The cla-bot has been summoned, and re-checked this pull request!

…#9453) - Make 'Add new repo...' footer extend full width of dropdown by using Flex::row with MainAxisSize::Max - Clear repo list hover state when footer is hovered by dispatching MenuAction::ClearHover(0) to the underlying Menu - Add ClearHover action to MenuAction enum to allow clearing hovered row index from external components Fixes warpdotdev#9453

- Add suppress_item_hover flag to SubMenu - Set suppress_item_hover=true on ClearHover, clear on ClearHoverDone - Check suppress_item_hover in render to disable hover highlighting - Add ClearHoverDone action to clear the suppress flag when pointer leaves footer - Update repo_picker.rs to dispatch ClearHoverDone when footer is no longer hovered Fixes warpdotdev#9453, addresses review feedback

…render chain - Fix on_hover callback in repo_picker.rs to use correct signature (is_hovered, ctx, app, position) - Pass suppress_item_hover flag through MenuItem::render() and MenuItemFields::render() - Update all render_submenus() calls to pass suppress_item_hover - Fixes review bot comments on PR warpdotdev#9477

ankit-songara · 2026-05-11T19:01:53Z

@cla-bot check

cla-bot · 2026-05-11T19:01:58Z

The cla-bot has been summoned, and re-checked this pull request!

github-actions · 2026-06-05T15:05:21Z

Hi @ankit-songara — a reviewer requested changes on this PR and it hasn't had activity from you in 24 days. When you get a chance, please push updates or reply to the review so a reviewer can take another look. Without activity, this PR will be automatically closed after 30 days of inactivity.

github-actions · 2026-06-07T12:48:50Z

Hi @ankit-songara — final reminder: a reviewer requested changes on this PR and it has been inactive for 26 days. It will be automatically closed in about 4 day(s) unless you push updates or reply. Maintainers can apply the no-autoclose label to keep it open.

oz-for-oss Bot requested changes Apr 29, 2026

View reviewed changes

moirahuang self-requested a review April 29, 2026 23:02

captainsafia added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label Apr 30, 2026 — with Warp Dev Github Integration

moirahuang removed their request for review April 30, 2026 19:53

oz-for-oss Bot requested changes Apr 30, 2026

View reviewed changes

ankit-songara force-pushed the fix/repo-selector-hover-states branch from fa9ce15 to d7318a9 Compare May 11, 2026 18:55

ankit-songara added 7 commits May 12, 2026 00:31

trigger CLA check

49543cc

fix: add ClearHover to action_accessibility_contents match

c679851

trigger re-review

93f7f8b

trigger fresh review (fix already applied in 6989548)

7235070

ankit-songara force-pushed the fix/repo-selector-hover-states branch from d7318a9 to bc263c2 Compare May 11, 2026 19:01

cla-bot Bot added the cla-signed label May 11, 2026

Conversation

ankit-songara commented Apr 29, 2026

Uh oh!

cla-bot Bot commented Apr 29, 2026

Uh oh!

oz-for-oss Bot commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot Apr 29, 2026

Choose a reason for hiding this comment

Uh oh!

ankit-songara Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

ankit-songara May 1, 2026

Choose a reason for hiding this comment

Uh oh!

cla-bot Bot commented Apr 29, 2026

Uh oh!

cla-bot Bot commented Apr 29, 2026

Uh oh!

ankit-songara commented Apr 29, 2026

Uh oh!

cla-bot Bot commented Apr 29, 2026

Uh oh!

cla-bot Bot commented Apr 29, 2026

Uh oh!

cla-bot Bot commented Apr 29, 2026

Uh oh!

moirahuang commented Apr 30, 2026

Uh oh!

cla-bot Bot commented Apr 30, 2026

Uh oh!

ankit-songara commented Apr 30, 2026

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

ankit-songara commented Apr 30, 2026

Uh oh!

cla-bot Bot commented Apr 30, 2026

Uh oh!

ankit-songara commented Apr 30, 2026

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

oz-for-oss Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

oz-for-oss Bot Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

cla-bot Bot commented May 1, 2026

Uh oh!

ankit-songara commented May 1, 2026

Uh oh!

ankit-songara commented May 3, 2026

Uh oh!

cla-bot Bot commented May 3, 2026

Uh oh!

cla-bot Bot commented May 3, 2026

Uh oh!

ankit-songara commented May 11, 2026

Uh oh!

oz-for-oss Bot commented Apr 29, 2026 •

edited

Loading