fix: button icon size contract + context menu touch selection - #696
Merged
Conversation
Button spinner/icon sizing moves to the doubled-selector + !important icon contract. The parent-scoped rules sat in the components layer, so consumers whose heroicons plugin emits into the utilities layer (the standard Phoenix setup, including petal.build) saw the mask's own w-6 width beat them regardless of specificity - md button icons rendered 24px wide instead of 20px. Context menu triggers suppress user-select under (pointer: coarse) so an iOS long press reads as the menu gesture instead of also starting text selection. Desktop selection inside the region is untouched.
Greptile SummaryThe PR updates button icon sizing to withstand consumer Heroicons cascade layers, suppresses context-menu text selection during touch long-press, stabilizes dropdown attribute order, and releases version 4.15.3.
Confidence Score: 4/5The PR should not merge until context-menu triggers preserve text selection when touch-primary hybrid devices are operated with a mouse or trackpad. The current primary-pointer media query still applies Files Needing Attention: assets/default.css
|
| Filename | Overview |
|---|---|
| assets/default.css | Button icon sizing now follows the established cascade-safe contract, but the context-menu media query still blocks selection on touch-primary hybrid devices using a mouse or trackpad. |
| lib/petal_components/dropdown.ex | Attribute collections now use keyword lists to preserve deterministic rendering order without changing their values. |
| mix.exs | Advances the package version from 4.15.2 to 4.15.3. |
| CHANGELOG.md | Documents the 4.15.3 release and its component fixes. |
Reviews (8): Last reviewed commit: "docs: changelog bullet for two_months in..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+9616
to
+9621
| @media (pointer: coarse) { | ||
| .pc-context-menu__trigger { | ||
| -webkit-user-select: none; | ||
| user-select: none; | ||
| } | ||
| } |
There was a problem hiding this comment.
When a touch-primary hybrid device is operated with a mouse or trackpad, (pointer: coarse) still applies user-select: none to the entire context-menu trigger, preventing users from selecting or copying its text despite the mouse-selection contract.
Greptile's catch: bare pointer:coarse also matches touch-primary hybrids currently driving a mouse. hover:none + pointer:coarse is the same true-touch guard the data table's mobile font sizing already uses.
…t-and-touch-select
4.15.2 published without this branch's two CSS fixes, so they move to their own 4.15.3 section and the version bumps to match. The avatar showcase refresh that did make 4.15.2 gets its changelog line.
| mid-hold otherwise. Guarded the same way as the data table's mobile | ||
| font sizing: hover:none + pointer:coarse means a true touch device, | ||
| so desktops (touchscreen laptops included) keep normal selection. */ | ||
| @media (hover: none) and (pointer: coarse) { |
There was a problem hiding this comment.
When a touch-primary device is operated with an attached mouse or trackpad, (hover: none) and (pointer: coarse) still matches the primary touchscreen and applies user-select: none, preventing users from selecting or copying text inside the context-menu trigger.
The panel's dynamic attributes came from a map, and Erlang's small-map iteration order is a compile-time property of the literal pool - a clean rebuild reorders them on its own, exactly as the old comment warned. CI's fresh build rendered phx-hook before style and the pinned legacy-default test went red on an untouched tree (the failure predates this branch: main went red at cc86b35, a version-bump-only commit). Keyword lists render in the order written, so the panel's HTML is now byte-stable across builds and the pin holds by construction.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #696 +/- ##
=======================================
Coverage 95.41% 95.42%
=======================================
Files 163 163
Lines 8078 8079 +1
=======================================
+ Hits 7708 7709 +1
Misses 370 370 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@greptileai review - two commits since your pass: the coarse-pointer guard you flagged is now (hover: none) and (pointer: coarse) per the data-table precedent, plus 4.15.3 staging and a dropdown attr-order determinism fix (the CI flake on main). |
Contributor
Author
|
@greptileai review |
nhobes
added a commit
that referenced
this pull request
Sep 1, 2026
The sweep #696's dropdown fix called for. Erlang's small-map iteration order is a compile-time property of the literal pool, so a clean rebuild of an untouched tree can reorder the attributes a map-backed HEEx splat renders - which is how CI went red on a pinned dropdown test. Every remaining helper whose result is splatted into a tag now returns a keyword list, rendered in the order written: - accordion's js_attributes carried the same multi-key hazard on its button (phx-click / aria-controls / aria-expanded) and content panel (id / role / aria-labelledby / style) - separator's aria helpers on the semantic vertical (role / aria-orientation) and labelled (role / aria-label) branches - menu's js_attributes clauses are single-attribute today; lists keep them order-stable if they ever grow Two helpers take maps from the caller rather than authoring one, so there is no written order to preserve - they sort instead, making the rendered order deterministic across builds: - toggle_group's slot passthrough (Map.drop on the slot entry) - pagination's phx-value-* set (Enum.map over event_values) Left alone deliberately: dropdown.ex (fixed on #696's branch), framework-built {@rest} globals, splats into component calls (assigns, never rendered order), and accordion's JS.dispatch detail payload (JSON read by key, not an attribute list). 1813 tests, 0 failures on a clean build before and after.
nhobes
added a commit
that referenced
this pull request
Sep 1, 2026
The sweep #696's dropdown fix called for. Erlang's small-map iteration order is a compile-time property of the literal pool, so a clean rebuild of an untouched tree can reorder the attributes a map-backed HEEx splat renders - which is how CI went red on a pinned dropdown test. Every remaining helper whose result is splatted into a tag now returns a keyword list, rendered in the order written: - accordion's js_attributes carried the same multi-key hazard on its button (phx-click / aria-controls / aria-expanded) and content panel (id / role / aria-labelledby / style) - separator's aria helpers on the semantic vertical (role / aria-orientation) and labelled (role / aria-label) branches - menu's js_attributes clauses are single-attribute today; lists keep them order-stable if they ever grow Two helpers take maps from the caller rather than authoring one, so there is no written order to preserve - they sort instead, making the rendered order deterministic across builds: - toggle_group's slot passthrough (Map.drop on the slot entry) - pagination's phx-value-* set (Enum.map over event_values) Left alone deliberately: dropdown.ex (fixed on #696's branch), framework-built {@rest} globals, splats into component calls (assigns, never rendered order), and accordion's JS.dispatch detail payload (JSON read by key, not an attribute list). 1813 tests, 0 failures on a clean build before and after.
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.
Two fixes, both CSS-only, both riding the unpublished 4.15.2.
Button icons render oversized in standard consumer apps. Nic spotted the loading/trailing icon on petal.build's buttons page sitting 24px wide vs the playground's 20px. Root cause is a cascade-layer fight, not specificity: consumer heroicons plugins (petal.build's
tailwind_heroicons.js, and the stock Phoenix setup) emit the mask's ownwidth: 1.5reminto@layer utilities, which outranks this library's@layer componentsrules no matter how specific the selector. (The playground only looked right because its heroicons land inbase.) The spinner/icon sizing rules now use the same doubled-selector +!importanticon contract as the pagination chevrons, combobox icons, etc.iOS long-press on a context menu region starts text selection. The trigger already killed the touch callout but not selection itself, so holding a card selected its text while the menu opened.
user-select: noneis now applied under@media (pointer: coarse)only - the hold reads purely as the menu gesture on touch, and mouse users keep normal text selection inside the region.Verified live on the playground: arrow icon computes 20x20 (unchanged), trigger reports
user-select: noneunder mobile emulation andautoon desktop. 1813 tests, 0 failures.