feat(status-area): honor SNI Status, attention icon, and live property changes#1427
Open
BrianHotopp wants to merge 4 commits into
Open
feat(status-area): honor SNI Status, attention icon, and live property changes#1427BrianHotopp wants to merge 4 commits into
BrianHotopp wants to merge 4 commits into
Conversation
mmstick
reviewed
May 29, 2026
| icon_theme_path: Option<PathBuf>, | ||
| /// Latest Status. Defaults to "Active" so an item is visible before its | ||
| /// first `IconUpdate` arrives — never hide an item that hasn't reported (P1). | ||
| status: String, |
Member
There was a problem hiding this comment.
This should be an enum that represents each possible state.
Member
|
There's a lot of unnecessary out of context commentary. Could you simplify the description of this PR and simplify the code comments? You must also include the GitHub PR template since that's mandatory. And make sure that each commit does one specific thing and they adhere to the requirements of the AI policy in that PR template. |
Member
|
Also, please include the PR template checklist. It is required. |
BrianHotopp
force-pushed
the
fix-sni-status-attention-signals
branch
from
09d50ce to
5d2f5a7
Compare
Contributor
Author
|
Thanks for the review. Simplified the description and code comments, added the PR template, and split into four atomic commits (test scaffolding, read Status + hide Passive, attention icon, refresh-on-change signals). Each commit builds and tests green on its own. |
BrianHotopp
force-pushed
the
fix-sni-status-attention-signals
branch
from
5d2f5a7 to
215d8ae
Compare
Extract `icon_events` to a module-level fn so it can be unit-tested, and add a fake StatusNotifierItem on a private session bus to drive the applet's real proxy against. Tests skip cleanly without a session bus; run with `dbus-run-session -- cargo test -p cosmic-applet-status-area`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The proxy never read Status, so a Passive item still consumed a panel slot. Read Status (defaulting to "Active" on a missing or throwing Get so a quirky-but-conformant item is never hidden) and filter Passive items out of every layout site through a single `visible_menus()`, keeping the icon/overflow positions and popup anchor math in sync. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AttentionIconName/AttentionIconPixmap were never read, so NeedsAttention alerts rendered blank or stale. Read them (defensively) and, via the pure `pick_icon_source`, prefer the attention icon only while status is "NeedsAttention" and one is present; otherwise fall through to the normal icon so the panel is never blank. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ath changes Icon refresh was driven only by NewIcon, so a runtime Status, attention, or IconThemePath change was missed. Merge all four signals via stream_select! and refetch on any of them; failed subscriptions log and drop instead of unwrapping, so one bad registration cannot abort the subscription task. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BrianHotopp
force-pushed
the
fix-sni-status-attention-signals
branch
from
215d8ae to
4b8bd8f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve StatusNotifierItem (SNI) handling in the status-area applet so tray icons behave correctly for items that vary their status or icon at runtime.
Statusproperty and hidePassiveitems so they no longer consume a panel slot. Status defaults toActiveon a missing or throwing Get, so quirky-but-conformant items (JetBrains, Dropbox) are never wrongly hidden.AttentionIconName/AttentionIconPixmapand show the attention icon while an item isNeedsAttention, falling back to the normal icon so the panel is never blank.NewStatus,NewAttentionIcon, andNewIconThemePath, not justNewIcon, so live property changes are picked up. A failed signal subscription is logged and dropped instead of unwrapping.Test plan
dbus-run-session -- cargo test -p cosmic-applet-status-area --lib→ 16 passed.