Skip to content

fix(content-sharing): style and callbacks#4480

Merged
mergify[bot] merged 1 commit intomasterfrom
fix-cs-style-and-callbacks
Mar 17, 2026
Merged

fix(content-sharing): style and callbacks#4480
mergify[bot] merged 1 commit intomasterfrom
fix-cs-style-and-callbacks

Conversation

@reneshen0328
Copy link
Copy Markdown
Contributor

@reneshen0328 reneshen0328 commented Mar 17, 2026

  • User see modernized style without pass in enableModernizedComponents prop in the wrapper
  • User see console logs when page cannot load (ex. invalid token) when pass in onError: (error) => {console.log('Failed to load sharing data:', error)} into the wrapper
  • User see console logs when invite a collaborator when pass in onClose: () => {console.log('Sharing modal closed')} into the wrapper

Summary by CodeRabbit

Release Notes

  • New Features

    • Added callback handlers for content sharing modal closure and error events during data loading.
  • Chores

    • Removed deprecated named export variant from the API.
    • Enabled modernized component rendering for improved performance.

@reneshen0328 reneshen0328 requested review from a team as code owners March 17, 2026 21:16
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 17, 2026

Walkthrough

The PR extends the ContentSharing component to add onClose and onError callback props, forward them to ContentSharingV2, import the ElementsXhrError type, remove a named export alias, and enable modernized components in the wrapper component.

Changes

Cohort / File(s) Summary
ContentSharing Component Enhancement
src/elements/content-sharing/ContentSharing.js
Added ElementsXhrError type import, introduced onClose and onError optional callback props to ContentSharingProps, destructured and forwarded both props to ContentSharingV2. Removed the named export alias ContentSharingComponent.
ContentSharing Wrapper Configuration
src/elements/wrappers/ContentSharing.js
Added enableModernizedComponents={true} prop to ContentSharingReactComponent in the render method.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

ready-to-merge

Suggested reviewers

  • tjuanitas
  • jfox-box

Poem

🐰 A rabbit hops through code with glee,
Callbacks flow from A to B,
Close and Error, hand in hand,
V2's modernized, oh so grand! 🎉

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main changes: enabling modernized styling and adding onClose/onError callbacks to the content-sharing component.
Description check ✅ Passed The description provides clear, testable acceptance criteria (checkboxes for three features) aligned with the PR objectives, though it lacks detailed implementation context.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-cs-style-and-callbacks
📝 Coding Plan
  • Generate coding plan for human review comments

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.

Tip

CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.

Add a .trivyignore file to your project to customize which findings Trivy reports.

Copy link
Copy Markdown
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.

Caution

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

⚠️ Outside diff range comments (1)
src/elements/wrappers/ContentSharing.js (1)

24-33: ⚠️ Potential issue | 🟠 Major

Move enableModernizedComponents after the spread to prevent caller override.

Currently, enableModernizedComponents={true} on line 25 can be overridden if this.options.enableModernizedComponents is provided, since props spread later take precedence. The wrapper should enforce modernized styling by default.

Proposed fix
         this.root.render(
             <ContentSharingReactComponent
-                enableModernizedComponents={true}
                 itemID={this.id}
                 itemType={itemType || ITEM_TYPE_FILE}
                 language={this.language}
                 messages={this.messages}
                 token={this.token}
                 uuid={uniqueId('contentSharing_')}
                 {...this.options}
+                enableModernizedComponents
             />,
         );
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/elements/wrappers/ContentSharing.js` around lines 24 - 33, The prop
enableModernizedComponents is currently placed before the props spread so
this.options.enableModernizedComponents can override it; update the JSX for
ContentSharingReactComponent so the spread {...this.options} comes first and
then set enableModernizedComponents={true} after the spread (keeping itemID,
itemType, language, messages, token, uuid as-is) to ensure the component always
receives the enforced modernized styling regardless of incoming options.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/elements/wrappers/ContentSharing.js`:
- Around line 24-33: The prop enableModernizedComponents is currently placed
before the props spread so this.options.enableModernizedComponents can override
it; update the JSX for ContentSharingReactComponent so the spread
{...this.options} comes first and then set enableModernizedComponents={true}
after the spread (keeping itemID, itemType, language, messages, token, uuid
as-is) to ensure the component always receives the enforced modernized styling
regardless of incoming options.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f4f9fc06-9514-4467-8221-ceb94a0abd7b

📥 Commits

Reviewing files that changed from the base of the PR and between 8e222b8 and e78f5f1.

📒 Files selected for processing (2)
  • src/elements/content-sharing/ContentSharing.js
  • src/elements/wrappers/ContentSharing.js

@tjuanitas
Copy link
Copy Markdown
Contributor

@Mergifyio queue

@tjuanitas
Copy link
Copy Markdown
Contributor

@Mergifyio refresh

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 17, 2026

Merge Queue Status

  • Entered queue2026-03-17 22:09 UTC · Rule: Automatic strict merge
  • Checks passed · in-place
  • Merged2026-03-17 22:09 UTC · at e78f5f1dedc190d4647d4fca2f477fbd4daede6f

This pull request spent 40 seconds in the queue, including 1 second running CI.

Required conditions to merge
  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • #review-threads-unresolved = 0 [🛡 GitHub branch protection]
  • branch-protection-review-decision = APPROVED [🛡 GitHub branch protection]
  • any of [🛡 GitHub branch protection]:
    • check-success = Summary
    • check-neutral = Summary
    • check-skipped = Summary
  • any of [🛡 GitHub branch protection]:
    • check-success = lint_test_build
    • check-neutral = lint_test_build
    • check-skipped = lint_test_build
  • any of [🛡 GitHub branch protection]:
    • check-success = license/cla
    • check-neutral = license/cla
    • check-skipped = license/cla
  • any of [🛡 GitHub branch protection]:
    • check-success = lint_pull_request
    • check-neutral = lint_pull_request
    • check-skipped = lint_pull_request

@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Mar 17, 2026

refresh

✅ Pull request refreshed

@mergify mergify bot added the queued label Mar 17, 2026
@mergify mergify bot merged commit 6154b5d into master Mar 17, 2026
15 checks passed
@mergify mergify bot deleted the fix-cs-style-and-callbacks branch March 17, 2026 22:09
@mergify mergify bot removed the queued label Mar 17, 2026
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.

3 participants