fix: order variable dropdowns by scale instead of alphabetically - #243
Conversation
Variables grouped per category were sorted with a plain natural sort, so scale families rendered in a lexicographic jumble (2xl, 2xs, 3xl, l, m, s, xl, xs) instead of small->large. This affected both the Bricks variable picker (injected into bricks_global_variables) and the Gutenberg token panel, which share Slashed_Inventory::get_variables_by_category(). Add a scale-aware comparator to Slashed_Category_Map (scale_order() + compare()/split_scale()) that groups tokens by base, orders the t-shirt scale (none, px, 2xs, xs, s, m, l, xl, 2xl, 3xl, 4xl...) by rank, and keeps numeric colour steps (50..950) numeric. get_variables_by_category() now sorts each category with usort() using this comparator. Fixes #232
|
Warning Review limit reached
Next review available in: 2 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe PR adds semantic ordering for scale and numeric variable suffixes. It groups related variable families and applies the new comparator to canonical and uncategorized inventory categories. PHPUnit tests cover scale, numeric, and family ordering. ChangesSemantic variable ordering
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This PR changes variable ordering to use semantic scale grouping and includes passing tests and static checks; no actionable merge-blocking risk remains. Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR replaces natural sorting of inventory variables with a shared semantic comparator for the Bricks and Gutenberg dropdowns.
Confidence Score: 4/5The PR should not merge until reserved suffixes are distinguished from identical words used by existing non-scale variables. The comparator currently shortens and ranks every variable ending in a known keyword, causing real inventory entries such as --sf-color-base and --sf-duration-none to move away from their natural family positions. Files Needing Attention: SLASHED-for-WP/includes/class-category-map.php
|
| Filename | Overview |
|---|---|
| SLASHED-for-WP/includes/class-category-map.php | Adds the semantic comparator, but globally interpreting reserved suffixes misclassifies existing non-scale variables and changes their family ordering. |
| SLASHED-for-WP/includes/class-inventory.php | Routes all inventory buckets through the new comparator, exposing its suffix classification behavior to both integrations. |
| tests-php/CategoryMapTest.php | Covers intended scale behavior but omits non-scale variables whose final segment collides with reserved keywords. |
Reviews (1): Last reviewed commit: "fix: order variable dropdowns by scale i..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| $scale = self::scale_order(); | ||
| $key = strtolower( $suffix ); | ||
| if ( isset( $scale[ $key ] ) ) { | ||
| return array( | ||
| 'base' => $base, | ||
| 'rank' => $scale[ $key ], | ||
| ); |
There was a problem hiding this comment.
When an existing non-scale variable ends in a reserved keyword, split_scale() unconditionally strips and ranks that suffix, causing entries such as --sf-color-base and --sf-duration-none to move away from their natural family positions in both integrations.
Addresses review feedback on the scale-ordering comparator. The first version stripped a trailing scale keyword and re-based every variable, which misclassified real tokens whose name merely contains a size-like word: --sf-color-base (the colour "base" family) was reparsed as a size of --sf-color and scattered from its own --sf-color-base-* steps, and --sf-duration-none jumped to the front of the duration keywords. Rework Slashed_Category_Map::compare() to walk names segment by segment, classing each segment as scale size / numeric step / plain word, so sizes order by rank, numbers numerically and everything else naturally. A name that is a prefix of another (bare family token vs its steps) sorts first. This is transitive (a strict weak ordering, unlike a mixed rank/string compare) and keeps size-word-named families contiguous and in place. Restrict scale_order() to unambiguous size keywords (px, 4xs…7xl) and drop the ambiguous none/base/full/max so they stay beside their family. Add regression tests for the --sf-color-base and --sf-duration-none cases. Refs #232
|
Addressed the Greptile P1 (reserved suffixes misclassifying tokens) in 452fcf5. Problem: the first comparator stripped a trailing scale keyword and re-based each variable, so tokens whose name merely contains a size-like word were misread as sizes:
Fix: Verified on the real inventory:
Added regression tests for both flagged cases. Full PHP suite: 240 passing; PHPStan clean; PHPCS clean on the changed source files. |
2 participants
Summary
Fixes #232 — variables in the admin dropdowns are now grouped and ordered by their semantic scale instead of alphabetically.
Within each category, variables were sorted with a plain natural sort, so scale families came out in a lexicographic jumble — e.g. Spacing rendered as
2xl, 2xs, 3xl, l, m, s, xl, xsinstead of small→large. This affected both integrations, since they shareSlashed_Inventory::get_variables_by_category():bricks_global_variablesoption, so the native Variable Manager picker renders in exactly that order.Change
Slashed_Category_Map:scale_order()— rank table for the t-shirt scale (none, px, 2xs, xs, s, m, l, xl, 2xl, 3xl, 4xl …) plus edge keywords (base,full,max).compare()/split_scale()— groups tokens by their "base" (name minus the trailing scale/numeric segment), orders scale members by rank, keeps numeric colour steps (50 … 950) numeric, and falls back to natural order for non-scale tokens so unrelated families stay grouped.get_variables_by_category()now sorts each category withusort()using this comparator (replacing the twosort(..., SORT_NATURAL | SORT_FLAG_CASE)calls).Result (real inventory)
none, px, 2xs, xs, s, m, l, xl, 2xl, 3xl, 4xl …none, 2xs, xs, s, m, l, xl, 2xl, 3xl, 4xl, full …--sf-color-primary*): bare token →50, 100, … 950→ variantsTests
tests-php/CategoryMapTest.phpcovering the t-shirt scale, edge/non-scale keywords, numeric colour steps, family grouping, and scale monotonicity.Notes
scripts/gen-bricks-inventory.jsstill usesnaturalComparefor the committedinventory.jsonfallback, but PHP re-sorts at runtime, so the dropdown order is fully corrected regardless.Summary by CodeRabbit
Improvements
Tests