feat(bricks): class documentation tooltips in builder - #180
Conversation
Add the editor-side consumer for the class-hints feature. A scoped, delegated hover listener inside the Bricks settings panel / class manager shows a styled tooltip with the class description + category, reading the data localized by class-rebemer-enqueue.php (window.slashedBricksEditor.showClassHints / classHints). - editor-app/src/lib/class-hints.js: pure resolveClassName() matcher plus DOM glue (scoped listener, styled tooltip, lifecycle). - main.js: init on start() (tied to the AbortController signal), destroy() on unload. - panel.css: .rebemer-class-hint tooltip styles. - tests/class-hints.test.js: unit tests for resolveClassName. - ci.yml: add classes-hints.json to the docs-freshness check. - rebuilt editor-app bundle. Co-Authored-By: Jack Granatowski <jack.granatowski@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR introduces a class documentation tooltip system for the Bricks builder editor. When users hover over class names in the editor, styled tooltips display class metadata (name, category, description) fetched from generated hints data. The implementation provides a pure resolver function, DOM-based tooltip display with viewport-aware positioning, delegated event handlers scoped to Bricks containers, and clean lifecycle management through init/destroy exports. ChangesClass hints tooltip feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/class-hints.js (1)
185-217: ⚡ Quick winOptional: hints are mouse-only — keyboard users can't trigger them.
The delegated listeners cover
mouseover/mouseoutbut there is no focus-based trigger, so users navigating the class list/autocomplete via keyboard never see the tooltip. Mirroring the hover handlers withfocusin/focusoutreusesfindHintTarget/show/hideand closes the gap with little added code.♿ Sketch: add focus handlers alongside hover
function onMouseOut(event) { if (!_currentTarget) return; // Hide only when the pointer truly leaves the labelled element. const to = event.relatedTarget; if (to instanceof Node && _currentTarget.contains(to)) return; hide(); } + +function onFocusIn(event) { + const target = event.target; + if (!(target instanceof Element)) return; + if (target.closest(`#${HOST_ID}`)) return; + if (!target.closest(BRICKS_CONTAINERS.join(','))) return; + const match = findHintTarget(target); + if (match && match.el !== _currentTarget) show(match.el, match.hint); +}Register
focusin/focusoutininit()with the sameopts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/class-hints.js` around lines 185 - 217, The hover-only hint logic means keyboard users won't see hints; add focus-based handlers that mirror the mouse handlers: implement focusin and focusout listeners (e.g., onFocusIn and onFocusOut) that call findHintTarget, show and hide exactly like onMouseOver/onMouseOut, reuse the same checks (HOST_ID, BRICKS_CONTAINERS, _currentTarget) and keep onKeyDown as-is for Escape; register these new listeners in init() alongside the existing mouseover/mouseout listeners using the same options so keyboard navigation triggers the same tooltip behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@plugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/class-hints.js`:
- Around line 185-217: The hover-only hint logic means keyboard users won't see
hints; add focus-based handlers that mirror the mouse handlers: implement
focusin and focusout listeners (e.g., onFocusIn and onFocusOut) that call
findHintTarget, show and hide exactly like onMouseOver/onMouseOut, reuse the
same checks (HOST_ID, BRICKS_CONTAINERS, _currentTarget) and keep onKeyDown
as-is for Escape; register these new listeners in init() alongside the existing
mouseover/mouseout listeners using the same options so keyboard navigation
triggers the same tooltip behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 60688609-d8d2-46d8-aec2-d72354ee3bd0
📒 Files selected for processing (8)
.github/workflows/ci.ymlpackage.jsonplugins/SLASHED-for-WP/integrations/bricks/assets/editor-app/app.cssplugins/SLASHED-for-WP/integrations/bricks/assets/editor-app/app.jsplugins/SLASHED-for-WP/integrations/bricks/editor-app/src/lib/class-hints.jsplugins/SLASHED-for-WP/integrations/bricks/editor-app/src/main.jsplugins/SLASHED-for-WP/integrations/bricks/editor-app/src/styles/panel.csstests/class-hints.test.js
Mirror the mouseover/mouseout handlers with focusin/focusout so users navigating the Bricks class list via keyboard get the same tooltip. Reuses findHintTarget/show/hide and the existing container scoping. Addresses CodeRabbit review on #180. Co-Authored-By: Jack Granatowski <jack.granatowski@gmail.com>
|
|
||
| // Mirror the hover handlers for keyboard users: a class label focused | ||
| // via Tab gets the same tooltip a mouse hover would surface. | ||
| function onFocusIn(event) { |
There was a problem hiding this comment.
Good call — added focusin/focusout handlers mirroring the hover logic (reusing findHintTarget/show/hide and the same BRICKS_CONTAINERS scoping), registered in init() with the same opts. Keyboard users tabbing through the class list now get the same tooltip. Done in db69f7d.
Summary
Completes the class documentation tooltips feature for the Bricks integration. All the data/admin/PHP plumbing already existed on
main(generator →data/classes-hints.json,show_class_hintssetting, REST, andclass-rebemer-enqueue.phplocalizingshowClassHints+classHints), but nothing on the editor side consumed it — so the feature was inert. This adds the missing editor-side consumer, built from scratch.When the "Show class hints" setting is on, hovering a SLASHED class inside the Bricks settings panel / class manager now shows a small styled tooltip with the class description and category.
What's added
editor-app/src/lib/class-hints.js— split into a pure matcher + thin DOM glue:resolveClassName(text, hints)— pure, DOM-free, unit-tested. Trims, strips a leading., requires a singlesf-*/is-*token that is an own-property of the hint map (rejects multi-token strings, so whole rows can't match; useshasOwnPropertysotoStringetc. aren't false hits).mouseover/mouseoutlistener scoped to known Bricks containers (#bricks-panel, class manager), never the body and never our own host; renders a styled tooltip into the existing#slashed-rebemer-host; hides onmouseout/scroll/Escape.init(enabled, hints, { signal })/destroy()lifecycle.main.js—classHints.init(cfg.showClassHints, cfg.classHints, { signal })instart()(tied to the existingAbortController),classHints.destroy()on unload.panel.css—.rebemer-class-hint*tooltip styles, reusing the reBEMer theme vars.tests/class-hints.test.js— 11node --testcases forresolveClassName; wired into thepretestscript alongsideelement-types.test.js.ci.yml— addsplugins/SLASHED-for-WP/data/classes-hints.jsonto thedocs-freshnessstaleness check (was previously uncovered).assets/editor-app/app.{js,css}).Why not the existing
feature/bricks-class-hints-…branchThat approach was a body-wide
mouseoverthat matched loosetextContent(including parents) and mutated Bricks' nodes via the nativetitleattribute. This implementation is scoped to the class-manager DOM, matches only exact known keys on the tightest element, and renders its own tooltip without touching Bricks' DOM.Notes
.githooks/pre-commit) rebuilds dist andgit addsdist/*.css, which.gitignoreexcludes, so it aborts every local commit underset -e. Tracked separately as a hook fix; it does not affect CI.Test plan
node --test tests/class-hints.test.js→ 11/11 pass.npm run docsleavesclasses-hints.jsonunchanged (freshness check passes).app.jsnow references the hint logic.Link to Devin session: https://app.devin.ai/sessions/eb845623267a4bebbb4e4fb6f634d160
Requested by: @jackgranatowski
Summary by CodeRabbit
New Features
Tests
Chores