close
Skip to content

fix(less): fix FA Kit icon alignment and spurious icon regressions#4409

Merged
imorland merged 1 commit into2.xfrom
im/fix-fa-kit-icon-alignment
Mar 7, 2026
Merged

fix(less): fix FA Kit icon alignment and spurious icon regressions#4409
imorland merged 1 commit into2.xfrom
im/fix-fa-kit-icon-alignment

Conversation

@imorland
Copy link
Copy Markdown
Member

@imorland imorland commented Mar 7, 2026

Summary

Fixes several icon layout issues introduced by FA Kit (SVG+JS mode) support and a regression from the .Button--icon LESS mixin conversion.

Root causes

  1. FA Kit injects a stylesheet dynamically (after Flarum's compiled CSS) that sets display: block via --fa-display on injected SVG elements. The previous fix used a broad display: inline-block !important on .Button-icon/.Button-caret globally, which then required counter-!important rules in Dropdown.less to hide elements — creating a cascade of overrides.

  2. d2129e63 converted .Button--icon to a LESS-only parametric mixin, removing its class-level CSS output. DOM elements with class="Button--icon" no longer had the caret/label hidden, causing the caret to appear in icon-only buttons (e.g. post controls ··· dropdown).

  3. UserCard controls dropdown was passing icon="fas fa-ellipsis-v" unconditionally. With 2.x's inline-flex Button layout the icon rendered visibly alongside the "Controls" label on the user profile page.

Fixes

  • Button.less: Scope the Kit display fix to .Button .Button-icon, .Button .Button-caret (no !important), placed before .Button--icon so hiding rules that follow override it by cascade order. Restore .Button--icon as a real CSS class selector.
  • Dropdown.less: Remove !important from split-dropdown hide/show rules (now handled by specificity/cascade). Add width: 16px !important on dropdown menu .Button-icon to prevent the Kit stylesheet overriding the fixed column width.
  • Input.less: Add width: 36px !important on .Input-prefix-icon to prevent the Kit stylesheet shifting the search icon left.
  • UserCard.js: Only pass the icon to the controls Dropdown when controlsButtonClassName includes Button--icon (icon-only context).

Test plan

  • Local font source: all buttons render correctly, no caret in ··· post controls, no ellipsis in UserPage "Controls" button, split dropdown shows caret only
  • CDN source: same as above
  • FA Kit source: same as above, plus search icon stays centred, split dropdown shows caret only (not ellipsis + caret)

🤖 Generated with Claude Code

FA Kit (SVG+JS mode) dynamically injects a stylesheet after Flarum's
compiled CSS, setting `display: block` via `--fa-display` on injected
SVG elements. This broke several layouts. Also fixes a regression from
d2129e6 where converting `.Button--icon` to a LESS-only parametric
mixin removed its class-level CSS output, causing the caret/label to
show in icon-only buttons.

Changes:

Button.less:
- Scope the FA Kit display fix to `.Button .Button-icon` and
  `.Button .Button-caret` (was a broad global rule with `!important`).
  Placed before `.Button--icon` so hiding rules that follow can
  override it by cascade order at equal specificity — no !important
  needed anywhere.
- Restore `.Button--icon` as a real CSS class selector (calls the
  mixin) so DOM elements with `class="Button--icon"` correctly hide
  the label and caret. This was a regression from d2129e6.

Dropdown.less:
- `.Dropdown--split .Dropdown-toggle .Button-icon`: was `display: none`
  but the old broad `!important` on `.Button-icon` prevented it from
  working, showing the ellipsis icon alongside the caret in split
  dropdown toggles. Now handled cleanly by cascade specificity.
- `.Dropdown-menu .Button-icon`: add `width: 16px !important` to
  prevent the FA Kit stylesheet from overriding the fixed column width
  with `width: 1.25em`.

Input.less:
- `.Input-prefix-icon`: add `width: 36px !important` to prevent the
  FA Kit stylesheet from overriding the positioning width, which was
  causing the search icon to shift left.

UserCard.js:
- Only pass `icon="fas fa-ellipsis-v"` to the controls Dropdown when
  `controlsButtonClassName` contains `Button--icon` (icon-only context).
  With 2.x's inline-flex Button layout, the icon was visibly rendered
  alongside the "Controls" label on the user profile page, which was
  not the intended design.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@imorland imorland requested a review from a team as a code owner March 7, 2026 15:27
@imorland imorland added this to the 2.0.0-beta.8 milestone Mar 7, 2026
@imorland imorland merged commit bf6b907 into 2.x Mar 7, 2026
34 checks passed
@imorland imorland deleted the im/fix-fa-kit-icon-alignment branch March 7, 2026 15:36
label={app.translator.trans('core.forum.user_controls.button')}
accessibleToggleLabel={app.translator.trans('core.forum.user_controls.toggle_dropdown_accessible_label')}
icon="fas fa-ellipsis-v"
icon={this.attrs.controlsButtonClassName?.includes('Button--icon') ? 'fas fa-ellipsis-v' : undefined}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to have caused the ellipsis to not show up at all for eg. user actions on mobile. Icon is no longer present on discuss (the caret one is, but hidden) and instead has a hidden "Controls" button label.

https://discuss.flarum.org/d/38800-flarum-200-beta7-released-rc-is-on-the-horizon/115

imorland added a commit that referenced this pull request Apr 13, 2026
Regression from bf6b907 (#4409), which conditionally removed the icon
prop from the UserHero controls Dropdown to prevent it rendering
alongside the label on desktop.

However, UserPage.less already has a rule hiding .Button-icon inside the
UserHero on desktop, and the @media @phone App-primaryControl rules
override it with display: block !important on mobile. The icon must
always be rendered for this CSS to work.

Revert the condition and always pass icon="fas fa-ellipsis-v".
imorland added a commit that referenced this pull request Apr 13, 2026
Regression from bf6b907 (#4409), which conditionally removed the icon
prop from the UserHero controls Dropdown to prevent it rendering
alongside the label on desktop.

However, UserPage.less already has a rule hiding .Button-icon inside the
UserHero on desktop, and the @media @phone App-primaryControl rules
override it with display: block !important on mobile. The icon must
always be rendered for this CSS to work.

Revert the condition and always pass icon="fas fa-ellipsis-v".
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants