Fix Dracula connecting-status color; harden getColorAttr#52
Merged
Conversation
…rden getColorAttr The 'Connecting…' / 'Testing…' status text fell into the colorOnPrimary branch, which renders dark on Dracula's light-purple primary instead of yellow. Add a statusConnectingColor attr (default colorOnPrimary; Dracula = yellow) and use it for the Connecting state. While wiring it up, surfaced and fixed a latent crash in getColorAttr: when a theme attr resolves to a literal color value (resourceId == 0) rather than a @color resource ref, ContextCompat.getColor(0) threw Resources$NotFoundException (Resource ID #0x0). Now use the literal data when it is a color type, falling back to TRANSPARENT otherwise. This hardens every getColorAttr caller, not just the new attr. Verified on-device (Dracula): connect no longer crashes; in-progress status renders yellow.
📝 WalkthroughWalkthroughAdds a new ChangesConnecting State Color Attribute
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
…hemes Per Greptile: Dracula forces night mode, so the active overrides live in values-night/themes.xml — which had statusConnected/Stopped but not the new statusConnectingColor, so connecting still inherited the dark base value in practice. Add statusConnectingColor=yellow to both night Dracula blocks, and to the API26+ base Theme.SagerNet in values-v26 (which redefines the status attrs) so the attr always resolves. Day-mode values were added in the previous commit.
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
1 participant
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.
Summary
Fixes the Dracula-theme discrepancy where the in-progress status text ("Connecting…",
"Testing…", "Stopping…") rendered dark/black instead of the theme's yellow — and fixes a
latent
getColorAttrcrash surfaced while wiring it up.Changes
statusConnectingColortheme attr. Default:?attr/colorOnPrimary(no change forother themes). Dracula:
@color/color_dracula_yellow. Use it for theConnectingstate inStatsBar.setStatusColorByState. PreviouslyConnectingfell into thecolorOnPrimarybranch,which resolves dark on Dracula's light-purple primary.
getColorAttr(ktx/Utils.kt): when a theme attr resolves to a literal colorvalue (
resourceId == 0) rather than a@colorresource reference,ContextCompat.getColor(0)threwResources$NotFoundException: Resource ID #0x0. Now use theliteral
datawhen it is a color type, else fall back toColor.TRANSPARENT. This protectsevery
getColorAttrcaller, not just the new attr.Why the crash
statusConnectingColorchained to?attr/colorOnPrimary, which some themes define as a literalcolor (no backing resource id). The old
getColorAttrblindly passedresourceId(0) togetColor, crashing on connect.Testing
renders yellow (verified "Testing…" / connecting states). Other themes unchanged (attr defaults
to
colorOnPrimary).Greptile for the final review.
Notes
connecting = yellow.
Greptile Summary
This PR fixes the Dracula theme showing dark/black text during connecting states and hardens
getColorAttragainst a crash when an attribute resolves to a literal color value rather than a color resource reference.statusConnectingColorattr: Declared inattrs.xml, defaulting to?attr/colorOnPrimaryin all themes and set to@color/color_dracula_yellowin all four Dracula variants (day/night × main/Dialog), with consistent coverage invalues-v26for API 26+.getColorAttrhardening (Utils.kt): Whentv.resourceId == 0(literal value, no backing resource), the old code passed0toContextCompat.getColor()causing aResources$NotFoundException. The fix checkstv.typeagainstTYPE_FIRST_COLOR_INT..TYPE_LAST_COLOR_INTand usestv.datafor literal colors, falling back toColor.TRANSPARENTfor unexpected types.StatsBar.setStatusColorByState:Connectingnow routes through the new custom attr instead of the barecolorOnPrimaryelsebranch.Confidence Score: 5/5
Safe to merge — targeted, well-scoped fix with no regressions on non-Dracula themes and full attr coverage across all theme variants.
The getColorAttr change correctly handles the literal-color case using the documented TypedValue type-range constants, and all four Dracula theme variants (day/night × main/Dialog) as well as the API-26 override have been updated in lock-step. Non-Dracula themes inherit the new attr from the base and keep existing colorOnPrimary behaviour unchanged.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[setStatusColorByState\nstate: BaseService.State] --> B{state?} B -->|Connected| C[R.attr.statusConnectedColor] B -->|Stopped / Stopping| D[R.attr.statusStoppedColor] B -->|Connecting NEW| E[R.attr.statusConnectingColor] B -->|else| F[material colorOnPrimary] C --> G[getColorAttr] D --> G E --> G F --> G G --> H{tv.resourceId != 0?} H -->|Yes| I[ContextCompat.getColor resourceId] H -->|No| J{tv.type in COLOR range?} J -->|Yes NEW| K[tv.data literal color value] J -->|No NEW| L[Color.TRANSPARENT safe fallback] I --> M[setTextColor] K --> M L --> M%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% flowchart TD A[setStatusColorByState\nstate: BaseService.State] --> B{state?} B -->|Connected| C[R.attr.statusConnectedColor] B -->|Stopped / Stopping| D[R.attr.statusStoppedColor] B -->|Connecting NEW| E[R.attr.statusConnectingColor] B -->|else| F[material colorOnPrimary] C --> G[getColorAttr] D --> G E --> G F --> G G --> H{tv.resourceId != 0?} H -->|Yes| I[ContextCompat.getColor resourceId] H -->|No| J{tv.type in COLOR range?} J -->|Yes NEW| K[tv.data literal color value] J -->|No NEW| L[Color.TRANSPARENT safe fallback] I --> M[setTextColor] K --> M L --> MReviews (2): Last reviewed commit: "fix(theme): add statusConnectingColor to..." | Re-trigger Greptile