Skip to content

Fix raw json field display in read only#16502

Merged
etiennejouan merged 5 commits intomainfrom
ej/16311
Dec 12, 2025
Merged

Fix raw json field display in read only#16502
etiennejouan merged 5 commits intomainfrom
ej/16311

Conversation

@etiennejouan
Copy link
Copy Markdown
Contributor

Preview
Screenshot 2025-12-11 at 15 54 07

Fixes #16311

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

2 issues found across 3 files

Prompt for AI agents (all 2 issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/twenty-front/src/modules/ui/layout/expandable-list/components/ExpandedFieldDisplay.tsx">

<violation number="1" location="packages/twenty-front/src/modules/ui/layout/expandable-list/components/ExpandedFieldDisplay.tsx:50">
P2: Click-outside detection may be incorrect. `refs.domReference` is the anchor element, not the floating popup. Consider also including the floating element ref to properly detect clicks outside the popup itself. Currently, clicking on the popup itself might incorrectly trigger `onClickOutside` since only the anchor is excluded.</violation>
</file>

<file name="packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/JsonFieldDisplay.tsx">

<violation number="1" location="packages/twenty-front/src/modules/object-record/record-field/ui/meta-types/display/components/JsonFieldDisplay.tsx:18">
P2: The click handler updates state unconditionally, but the expanded view only renders when `isRecordFieldReadOnly` is true. This causes unnecessary re-renders and may confuse users when clicking has no visible effect. Consider guarding the state update with the same condition.</violation>
</file>

Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR

});

useListenClickOutside({
refs: [refs.domReference],
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Dec 11, 2025

Choose a reason for hiding this comment

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

P2: Click-outside detection may be incorrect. refs.domReference is the anchor element, not the floating popup. Consider also including the floating element ref to properly detect clicks outside the popup itself. Currently, clicking on the popup itself might incorrectly trigger onClickOutside since only the anchor is excluded.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/twenty-front/src/modules/ui/layout/expandable-list/components/ExpandedFieldDisplay.tsx, line 50:

<comment>Click-outside detection may be incorrect. `refs.domReference` is the anchor element, not the floating popup. Consider also including the floating element ref to properly detect clicks outside the popup itself. Currently, clicking on the popup itself might incorrectly trigger `onClickOutside` since only the anchor is excluded.</comment>

<file context>
@@ -0,0 +1,68 @@
+  });
+
+  useListenClickOutside({
+    refs: [refs.domReference],
+    callback: () =&gt; {
+      onClickOutside?.();
</file context>

✅ Addressed in 4d25f4a

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Dec 11, 2025

🚀 Preview Environment Ready!

Your preview environment is available at: http://bore.pub:2013

This environment will automatically shut down when the PR is closed or after 5 hours.

Copy link
Copy Markdown
Contributor

@Devessier Devessier left a comment

Choose a reason for hiding this comment

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

Great job, Mister Proust 💪

@etiennejouan etiennejouan merged commit c274adf into main Dec 12, 2025
69 checks passed
@etiennejouan etiennejouan deleted the ej/16311 branch December 12, 2025 10:27
@twenty-eng-sync
Copy link
Copy Markdown

Hey @etiennejouan! After you've done the QA of your Pull Request, you can mark it as done here. Thank you!

NotYen pushed a commit to NotYen/twenty-ym that referenced this pull request Dec 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Can't open workflow run state

2 participants