Repository navigation
#41 — feat(renderer): DAG keyboard accessibility + #40 renderer hygiene - #49
Conversation
Bundles cute-dbt#41 (DAG node keyboard accessibility) with the lightweight subset of cute-dbt#40's post-merge bot findings. The Gemini-2+4 sqlparser AST refactor is deferred to a dedicated follow-up PR — #40 stays open until that ships. #41 — DAG nodes now tab-focusable with role="button", populated aria-label, and Enter/Space keydown handlers that share the same activation closure as click. Focus-visible outline isolates keyboard focus from mouse clicks. Subtitle copy updated to mention Tab+Enter. #40 CR-2/3/4/5/6: - Dropped dead NodePayload::ref_name (template never read node.ref). - Dropped unused `t` param from buildMermaidSource(m, t). - Hoisted mermaid.initialize to the startup block (one call total rather than per-renderDag). - Added visible .format-notice for fixtures with format: csv / sql (previously rendered as silently empty tables). - Dropped unused _model_name param from find_import_node_id; the caller (build_test_payload) no longer threads it either. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
Warning Review limit reached
Your plan currently allows 1 review/hour. Refill in 50 minutes and 49 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (3)
✨ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves keyboard accessibility for the CTE dependency DAG and adds support for displaying notices when non-array fixture formats (CSV, SQL) are used. It also refactors the Rust adapter to remove unused fields and optimizes Mermaid.js initialization. Feedback suggests restoring focus to the selected node after the DAG re-renders to ensure a smooth keyboard navigation experience and removing hardcoded version strings from user-facing notices to simplify future maintenance.
| window.mermaid.render(id, src).then(function (out) { | ||
| $host.html(out.svg); |
There was a problem hiding this comment.
When a DAG node is activated via keyboard, the renderDag() call re-renders the entire SVG, which causes the browser to lose focus on the currently active element. To maintain a good keyboard navigation experience, focus should be restored to the newly rendered node that matches state.selectedNodeId after the DOM is updated. Since renderDag is asynchronous and the nodes are bound in a subsequent step, you'll need to ensure focus is restored after bindDagNodeClicks completes.
There was a problem hiding this comment.
Fixed — see 68d0a52. The diagnosis is correct: renderDag() rebuilds the SVG via mermaid.render → $host.html(out.svg), which detaches every previously focused element.
Fix shape: at the end of bindDagNodeClicks(svgHost, ...), after the per-node loop, look up g.dag-node[data-node-id="<state.selectedNodeId>"] in the freshly rendered SVG and call .focus({ preventScroll: true }) on it. Defensive escape on the selector value handles any CTE name with a quote (rare but possible).
Specifically did not track an activatedViaKeyboard flag — :focus-visible already reflects the originating input heuristic, so programmatic .focus() after a click event leaves the keyboard/pointer signal untouched. A mouse activation will not draw a stuck outline.
Verified via Playwright: focus a node, dispatch Enter, wait for re-render — document.activeElement is the new g.dag-node with the same data-node-id after the SVG was wholly replaced.
| return $("<div>").addClass("format-notice") | ||
| .append($("<span>").text("Fixture uses ")) | ||
| .append($("<code>").text("format: " + format)) | ||
| .append($("<span>").text(" — not yet visualized in cute-dbt v0.1. See the raw fixture in the manifest.")); |
There was a problem hiding this comment.
The version string v0.1 is hardcoded in the user-facing notice. To avoid maintenance overhead when the project version is bumped, consider removing the specific version number or using a variable if available.
| .append($("<span>").text(" — not yet visualized in cute-dbt v0.1. See the raw fixture in the manifest.")); | |
| .append($("<span>").text(" — not yet visualized in cute-dbt. See the raw fixture in the manifest.")); |
There was a problem hiding this comment.
Fixed — see 68d0a52. Suggestion adopted essentially verbatim; the notice now reads:
Fixture uses
format: csv— not yet visualized in cute-dbt. See the raw fixture in the manifest.
Reasoning: per the cute-dbt phased roadmap, a future minor (v0.2 or v0.3) may add format-aware rendering, at which point this notice goes away entirely. Anchoring on "v0.1" specifically would either rot or force a copy-edit every minor; dropping the version is the lighter-weight maintenance posture.
…d v0.1 Addresses Gemini's two MEDIUM findings on PR 49: - bindDagNodeClicks now refocuses the g.dag-node matching state.selectedNodeId after every render. The previous keyboard activation path lost focus because renderDag() rebuilds the SVG and detaches the focused element. :focus-visible reflects the originating input heuristic, so mouse activation still won't draw a stuck outline. - buildFormatNotVisualizedNotice no longer hardcodes "v0.1" in the user-facing copy. If v0.2 ships format-aware rendering this notice goes away entirely; the version string adds maintenance overhead without semantic value. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
1 participant
Summary
Bundles cute-dbt#41 (DAG node keyboard accessibility) with the
lightweight subset of cute-dbt#40's post-merge bot findings. The
substantial Gemini-2+4 sqlparser AST refactor is deferred to a
dedicated follow-up PR — #40 stays open until that ships.
Closes / refs
Closes #41— DAG node keyboard accessibilityNodePayload::ref_namefield (template never read it)tparameter frombuildMermaidSource(m)mermaid.initializeto the$(function)startup blockformat: csv/format: sql(previously rendered an empty table silently)
_model_nameparameter fromfind_import_node_id(the caller no longer threads it either)
is_simple_from_select+extract_table_leaf_refs. That change needs anengine-API decision (renderer re-parses the body slice as a standalone
statement, vs.
CteNodeexposing aQueryAST accessor) and deservesits own focused review.
#41 — DAG keyboard accessibility
g.dag-nodeSVG group getstabindex="0",role="button", and anaria-labelof the formInspect DAG node: <id>.activate()closure replaces the inline click handler; it is boundto both
clickand akeydownhandler that fires onEnterandSpace(with
preventDefaulton Space so the page doesn't scroll).g.dag-node:focus-visible { outline: 2px dashed #1d7484; outline-offset: 3px; }isolates keyboard focus from mouse clicks so a click doesn't leave a ring
stuck on the clicked node.
Click a node (or Tab to it and press Enter) to inspect.#40 CR-5 — fixture-format notice
When a
UnitTestGiven/UnitTestExpecthasformatset tocsvorsql, the underlyingrowsis a string rather than the column-keyedarray the table renderer expects. Previously
unwrapRowssilentlyreturned
[]and the user saw an empty table. NowisNonArrayFormatFixture(obj)detects the case and the render path emits a
.format-noticedivexplaining that the fixture is intentional but not yet visualized in
cute-dbt v0.1.
Verification
cargo nextest run— 315 / 315 passing (snapshots rebaselined)cargo test --test bdd— 27 scenarios / 153 steps passing (headlesszero-egress + resource-ref lint both green; new CSS/JS introduces no
external refs)
crap4rs --coverage lcov.info— PASS (0 functions above threshold;worst 11.0, well under strict-15 on
src/adapters/render.rs)cargo deny check— advisories ok / bans ok / licenses ok / sources okhttp://127.0.0.1:8742/jaffle-shop-report.html:tabindex="0",role="button", populatedaria-label)target.focus(); keyboard.press('Enter')activates the node-detailpanel (verified
data-node-idon.node-detailmatches the focusednode)
keydownwithkey=' 'activates withdefaultPrevented: truefocus-visiblecomputed style matches the rule(
2px dashed rgb(29,116,132), offset3px).format-noticeinjected into the DOM picks up the right computedbackground / border / code styling
examples/jaffle-shop-report.htmlregenerated for the byte-equality gate.Test plan
render.rsbot review is worth waiting for here vs. PR 11.5 which was merged under
CR rate-limiting)
🤖 Generated with Claude Code