[Call node] Add support for href with action - #16
Conversation
|
Warning Review limit reached
Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds an ChangesEmbedded Flow Action Wiring
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant RichTextContainer
participant AuthOptionFactory
participant OnSubmitCallback
User->>RichTextContainer: click anchor element
RichTextContainer->>AuthOptionFactory: click event bubbles to handler
AuthOptionFactory->>AuthOptionFactory: resolve anchor and data-action-ref
AuthOptionFactory->>AuthOptionFactory: prevent default and build formData
AuthOptionFactory->>OnSubmitCallback: onSubmit(synthesized Action, formData)
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/react/src/components/presentation/auth/__tests__/AuthOptionFactory.test.tsx (1)
24-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
richTextWithLinkaccepts a loosely-typed inlineactionshape instead ofEmbeddedFlowComponentAction.
action?: {ref: string; eventType?: string}duplicates the shape of the exportedEmbeddedFlowComponentActionmodel instead of importing and reusing it. Minor drift risk if the model changes.🤖 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 `@packages/react/src/components/presentation/auth/__tests__/AuthOptionFactory.test.tsx` around lines 24 - 29, The rich text test helper is duplicating the inline action shape instead of using the shared model type. Update richTextWithLink in AuthOptionFactory.test.tsx to import and use EmbeddedFlowComponentAction for the action parameter, keeping its signature aligned with EmbeddedFlowComponent and avoiding type drift if the model changes.packages/react/src/components/presentation/auth/AuthOptionFactory.tsx (1)
561-564: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
formData-building loop.This loop that copies
formValuesintoformDataduplicates the identical logic in theActioncase'shandleClick(Lines 309-313). Consider extracting a small shared helper (e.g.buildFormData(formValues)) to avoid drift between the two copies.♻️ Proposed helper extraction
+const buildFormDataFromValues = (formValues: Record<string, string>): Record<string, string> => { + const formData: Record<string, string> = {}; + Object.keys(formValues).forEach((field: string) => { + formData[field] = formValues[field]; + }); + return formData; +}; + const createAuthComponentFromFlow = (🤖 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 `@packages/react/src/components/presentation/auth/AuthOptionFactory.tsx` around lines 561 - 564, The formData copy loop in AuthOptionFactory is duplicated and should be centralized to avoid drift between the Action case and the current path. Extract the shared Object.keys(formValues) -> formData mapping into a small helper such as buildFormData(formValues), then call that helper from both handleClick and the other form submission flow so there is only one implementation to maintain.
🤖 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.
Inline comments:
In `@packages/react/src/components/presentation/auth/AuthOptionFactory.tsx`:
- Around line 570-577: Sentinel anchors handled by AuthOptionFactory’s rich text
click delegation are not keyboard-accessible when they lack href, so the current
eslint-disable justification is incorrect. Update the anchor authoring/handling
contract in AuthOptionFactory so sentinel anchors are either required to include
a non-empty href (or another keyboard-accessible default) or are made focusable
and operable with keyboard handlers in addition to onClick. Use the rich-text
click path and its sentinel-anchor detection logic in AuthOptionFactory to
locate the fix.
- Around line 527-568: The rich text click fallback in AuthOptionFactory’s
handleRichTextClick currently hijacks every anchor when no data-action-ref
sentinel is present. Tighten the fallback so it only dispatches the action in
unambiguous cases, such as when the container has exactly one anchor, and
otherwise ignore the click unless the anchor’s data-action-ref matches
richTextAction.ref. Update the related tests that currently expect “binds to any
anchor when no sentinel-marked anchor exists” to cover the safer behavior.
---
Nitpick comments:
In
`@packages/react/src/components/presentation/auth/__tests__/AuthOptionFactory.test.tsx`:
- Around line 24-29: The rich text test helper is duplicating the inline action
shape instead of using the shared model type. Update richTextWithLink in
AuthOptionFactory.test.tsx to import and use EmbeddedFlowComponentAction for the
action parameter, keeping its signature aligned with EmbeddedFlowComponent and
avoiding type drift if the model changes.
In `@packages/react/src/components/presentation/auth/AuthOptionFactory.tsx`:
- Around line 561-564: The formData copy loop in AuthOptionFactory is duplicated
and should be centralized to avoid drift between the Action case and the current
path. Extract the shared Object.keys(formValues) -> formData mapping into a
small helper such as buildFormData(formValues), then call that helper from both
handleClick and the other form submission flow so there is only one
implementation to maintain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a09ed46b-20d7-455f-95ed-a892e77479ab
📒 Files selected for processing (5)
packages/javascript/src/index.tspackages/javascript/src/models/__tests__/embedded-flow.test.tspackages/javascript/src/models/embedded-flow.tspackages/react/src/components/presentation/auth/AuthOptionFactory.tsxpackages/react/src/components/presentation/auth/__tests__/AuthOptionFactory.test.tsx
f87ed06 to
b2950ff
Compare
b2950ff to
3ca1fcc
Compare
Purpose
This pull request introduces support for optional action wiring on rich-text components in embedded flows, allowing specific anchor tags within sanitized HTML to trigger flow actions. The changes include new TypeScript interfaces, enhancements to the React component for handling these actions, and comprehensive unit tests to ensure correct behavior.
Approach
Embedded Flow Model Enhancements:
EmbeddedFlowComponentActioninterface to represent optional actions that can be wired to otherwise passive components, such asRICH_TEXT. This enables anchor tags with adata-action-refattribute to dispatch flow actions.EmbeddedFlowComponentinterface to include an optionalactionproperty, allowing components to specify action wiring.EmbeddedFlowComponentActiontype from the main JavaScript package entry point.React Component Updates:
AuthOptionFactory.tsxto handle the newactionproperty on rich-text components. It now intercepts clicks on sentinel-marked anchors, dispatching a synthetic action with the correct event type and validation semantics. [1] [2]Testing:
Related Issues
Related PRs
Checklist
breaking changelabel added.Security checks
Summary by CodeRabbit
New Features
EmbeddedFlowComponentActiontype for consistent use across the library.Bug Fixes
Tests