fix: show vertical tab notification dots when inbox is hidden#11611
Conversation
Co-Authored-By: Oz <oz-agent@warp.dev> Co-Authored-By: Zach Lloyd <zachlloyd@users.noreply.github.com>
|
@oz-agent i think there is a second issue here, which is that the "summary" view of a tab doesn't render the notification dot. please fix that |
|
I'm working on changes requested in this PR (responding to a PR conversation comment). You can view the conversation on Warp. I pushed changes to this PR based on the comment. Next steps:
Powered by Oz |
Render unread notification activity in summary-mode vertical tab rows by carrying the unread flag in summary data. Co-Authored-By: Zach Lloyd <zachlloyd@users.noreply.github.com> Co-Authored-By: Oz <oz-agent@warp.dev>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
| }); | ||
| } | ||
|
|
||
| let notifications_placed = placed.contains(&&HeaderToolbarItemKind::NotificationsMailbox); |
There was a problem hiding this comment.
This deletion seems incorrect? Think this should still be propagating the setting flag
There was a problem hiding this comment.
why should disabling the notifications mailbox have any effect on whether we show in app notifications? that's one of the bugs i'm trying to fix. maybe i'm misunderstanding the code?
There was a problem hiding this comment.
I guess that’s fair. U could still independently disable this setting if u wantsd
There was a problem hiding this comment.
i think that's the right behavior. it was unexpected to me that disabling the inbox turned off notifications entirely
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR keeps agent notification records available for vertical-tab unread indicators even when the in-app notification UI is hidden, stops toolbar customization from mutating the notification visibility setting, and adds unit coverage for unread tracking while notifications are disabled.
Concerns
None.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
3 participants
Closes #11497
Summary
show_agent_notificationswhen the Notifications mailbox button is removed.https://www.loom.com/share/c29bc7267e1b468d849feff52dfc3aea
Validation
cargo fmtcargo test -p warp ai::agent_management::agent_management_model::tests --libcargo clippy -p warp --lib --tests -- -D warnings