Skip to content

Vlad/ab 807#1271

Merged
UladzislauK-Writer merged 2 commits intodevfrom
vlad/AB-807
Jan 23, 2026
Merged

Vlad/ab 807#1271
UladzislauK-Writer merged 2 commits intodevfrom
vlad/AB-807

Conversation

@UladzislauK-Writer
Copy link
Collaborator

@UladzislauK-Writer UladzislauK-Writer commented Jan 23, 2026

Summary by CodeRabbit

  • Refactor

    • Simplified the dataframe and tag components by removing conditional interactivity wiring, streamlining initialization and event handling for cleaner, more maintainable behavior.
    • Event emission paths simplified so interactions are handled consistently across components.
  • Style

    • Tag elements now display a consistent pointer cursor to better indicate interactivity.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 23, 2026

📝 Walkthrough

Walkthrough

This PR simplifies event handling by removing component/instance injections and blueprint checks from the dataframe/tag components and the dataframe value broker; the broker now accepts only an emitter element and table, and events are emitted unconditionally.

Changes

Cohort / File(s) Summary
Dataframe hook parameter refactoring
src/ui/src/components/core/content/CoreDataframe/useDataframeValueBroker.ts
Removed Core and InstancePath params from the hook signature; removed component lookup, useComponentLinkedBlueprints usage, and isEventUsed gating. Handlers now dispatch events unconditionally; signature simplified to (emitterEl, table).
Component hook consumption updates
src/ui/src/components/core/content/CoreDataframe.vue
Removed injections for core and instancePath; updated useDataFrameValueBroker call to the new single-argument form (emitterEl, table) and adjusted destructured handlers sourcing accordingly.
Blueprint integration removal (tags)
src/ui/src/components/core/content/CoreTags.vue
Removed componentId and core injections and useComponentLinkedBlueprints usage; removed isClickable computed gating and replaced dynamic cursor logic with a constant pointer. Tag click events still dispatched as wf-tag-click.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant TagComponent as Tag Component
    participant DataframeComponent as Dataframe Component
    participant Broker as useDataFrameValueBroker
    participant Host as Host Element / Parent

    rect rgba(135,206,250,0.5)
    TagComponent->>Broker: tag click -> handler (emitterEl)
    Broker->>Host: dispatchEvent('wf-tag-click', payload)
    end

    rect rgba(144,238,144,0.5)
    DataframeComponent->>Broker: cell/edit/add/action -> handler (emitterEl, table)
    Broker->>Host: dispatchEvent('wf-dataframe-event', payload)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • madeindjs

Poem

🐰 A broker freed from old constraints,

No blueprints slowing event complaints.
Clicks and edits hop straight through,
A simpler path, a cleaner view.
Hooray — the rabbit cheers anew! 🥕

🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Vlad/ab 807' is vague and does not convey meaningful information about the changes; it appears to be a branch name rather than a descriptive PR title. Use a descriptive title that summarizes the main change, such as 'Fix dataframe and tag events in deployed mode' or 'Enable dataframe events dispatching in run mode'.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The PR addresses the core issue by removing component-linked blueprint logic and unconditionally dispatching dataframe events, which should enable events in deployed mode matching preview behavior.
Out of Scope Changes check ✅ Passed All changes are directly related to the linked issue: removing blueprint linkage logic and unconditionally dispatching events to fix deployed mode behavior.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/ui/src/components/core/content/CoreDataframe/useDataframeValueBroker.ts (1)

44-48: Missing null check for table.value in handlerAddRow.

Unlike handlerActionRow (line 82) and handlerUpdateCell (line 103), this function accesses table.value.numRows() without first checking if table.value is null. This inconsistency could cause a runtime error if called before the table is initialized.

🐛 Proposed fix
 async function handlerAddRow() {
 	const eventType = "wf-dataframe-add";
+	if (!table.value) throw Error("Table is not ready");
 
 	const rowIndex = table.value.numRows();
🤖 Fix all issues with AI agents
In `@src/ui/src/components/core/content/CoreTags.vue`:
- Line 176: CoreTags.vue currently sets cursor: pointer unconditionally on tags
causing non-interactive tags to appear clickable; change the template/style to
apply cursor: pointer only when a click handler is present (e.g., check for the
wf-tag-click listener or a prop like clickable) and fall back to default cursor
otherwise. Locate the tag element and update its binding (use a conditional
class or inline style tied to the presence of $listeners['wf-tag-click'] or a
boolean prop) so the cursor is pointer only when wf-tag-click is registered or
when the component's clickable flag is true.

align-items: center;
gap: 4px;
cursor: v-bind("isClickable ? 'pointer' : 'auto'");
cursor: pointer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Cursor is now always pointer, even without a registered handler.

Previously, the cursor style was conditional on whether a click handler was registered. Now tags always display a pointer cursor, which may mislead users into expecting interactivity when no wf-tag-click handler is attached.

Consider whether this UX change is intentional or if you want to retain conditional cursor styling through a different mechanism (e.g., a field option to enable/disable clickability).

🤖 Prompt for AI Agents
In `@src/ui/src/components/core/content/CoreTags.vue` at line 176, CoreTags.vue
currently sets cursor: pointer unconditionally on tags causing non-interactive
tags to appear clickable; change the template/style to apply cursor: pointer
only when a click handler is present (e.g., check for the wf-tag-click listener
or a prop like clickable) and fall back to default cursor otherwise. Locate the
tag element and update its binding (use a conditional class or inline style tied
to the presence of $listeners['wf-tag-click'] or a boolean prop) so the cursor
is pointer only when wf-tag-click is registered or when the component's
clickable flag is true.

@UladzislauK-Writer UladzislauK-Writer merged commit 9a99eb4 into dev Jan 23, 2026
17 checks passed
@UladzislauK-Writer UladzislauK-Writer deleted the vlad/AB-807 branch January 23, 2026 10:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants