[2.x] fix: keep icons whose style carries meaning out of the forced style - #4873
Merged
Conversation
An admin forcing a FontAwesome style asks for uniform icons, but a few icons use their weight to say something. A filled circle means a user is online; a solid star means a discussion is followed while a regular one means it isn't. Restyling those to a single weight either turns the dot into a thin ring or makes both stars identical, so following and not following look the same. Those icons now opt out with noStyleOverride: the online indicator (both the helper and the copy in UserCard), and the subscription stars in the menu and the discussion controls. DetailedDropdownItem accepted only an icon name and wrapped it itself, leaving no way to pass the flag through, so it now also takes a rendered icon — matching what Button already allows. Icons that pair distinct glyphs rather than weights (tags' toggle uses fa-check-circle against fa-circle) restyle safely and are left alone.
imorland
added a commit
that referenced
this pull request
Aug 2, 2026
…4878) The discussion list signals unread replies by icon weight alone: a solid comment for unread against a regular one for read. Forcing a FontAwesome style restyled both to the same weight, making the two states indistinguishable. The unread icon now opts out with noStyleOverride, the same exemption the online indicator and subscription stars use (#4873). The read-state icon still follows the forced style, so the contrast survives — the checkmark shown on hover is an action glyph, not a weight signal, and keeps following the forced style too.
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
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.
Follow-up to #4868, which added the forced FontAwesome style setting along with a
noStyleOverrideescape hatch for icons that shouldn't be restyled. This applies that hatch where it's actually needed.Changes proposed in this pull request
An admin who forces a style is asking for uniform icons — but a few icons use their weight to convey state, and flattening those loses information rather than restyling it:
fa-solid fa-circle)fa-light/fa-duotone; "online" stops reading as onlinefa-regularvsfa-solid fa-star)Fixed by opting those out:
common/helpers/userOnlineandforum/components/UserCard— the online dot (two separate copies of it).SubscriptionMenu(menu rows and the split-button) andaddSubscriptionControls(the discussion controls item).DetailedDropdownItemneeded a small widening to make this possible: it typediconasstringand wrapped it in<Icon>itself, so there was no way to pass an attribute through. It now acceptsstring | Mithril.Childrenand renders a passed element as-is — the same contractButtonalready offers.What was deliberately left alone
I swept core and the bundled extensions for state-carrying icons. Pairs that use distinct glyphs rather than weights restyle perfectly safely and are untouched — tags'
ToggleButton(fa-check-circlevsfa-circle), package-manager'sfa-check-circle/fa-circle-down, likes' thumbs (decorative, single-state), and subscriptions' notification/filter icons (single-state labels).Verification
Live on a forum with
fa-duotone fa-lightforced: the stars keep their own weights in the DOM (icon fa-regular fa-staralongsideicon fa-solid fa-star), so the follow state stays legible while every other icon takes the forced style. Core's JS suite (294 tests) passes, typings check clean in both packages, subscriptions builds.Worth stating plainly: this is a limitation inherent to the feature, not a defect in it. Any icon whose weight is semantic needs the opt-out, so if reviewers know of others I've missed, they're one prop away from being fixed.
Confirmed
yarn jest), typings check passes.