Skip to content

fix(feedback): Override TriggerLabel Option #12316

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 31, 2024
Merged

fix(feedback): Override TriggerLabel Option #12316

merged 2 commits into from
May 31, 2024

Conversation

c298lee
Copy link
Contributor

@c298lee c298lee commented May 31, 2024

Fixes #12229

@c298lee c298lee requested a review from a team as a code owner May 31, 2024 14:22
Copy link
Contributor

github-actions bot commented May 31, 2024

size-limit report 📦

Path Size
@sentry/browser 21.74 KB (0%)
@sentry/browser (incl. Tracing) 32.76 KB (0%)
@sentry/browser (incl. Tracing, Replay) 68.33 KB (0%)
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags 61.64 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) 72.39 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) 84.49 KB (-0.01% 🔽)
@sentry/browser (incl. Tracing, Replay, Feedback, metrics) 86.35 KB (-0.01% 🔽)
@sentry/browser (incl. metrics) 25.92 KB (0%)
@sentry/browser (incl. Feedback) 37.89 KB (-0.02% 🔽)
@sentry/browser (incl. sendFeedback) 26.32 KB (0%)
@sentry/browser (incl. FeedbackAsync) 30.86 KB (-0.01% 🔽)
@sentry/react 24.51 KB (0%)
@sentry/react (incl. Tracing) 35.8 KB (0%)
@sentry/vue 25.73 KB (0%)
@sentry/vue (incl. Tracing) 34.59 KB (0%)
@sentry/svelte 21.87 KB (0%)
CDN Bundle 23.11 KB (0%)
CDN Bundle (incl. Tracing) 34.5 KB (0%)
CDN Bundle (incl. Tracing, Replay) 68.42 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) 73.59 KB (-0.01% 🔽)
CDN Bundle - uncompressed 68 KB (0%)
CDN Bundle (incl. Tracing) - uncompressed 102.16 KB (0%)
CDN Bundle (incl. Tracing, Replay) - uncompressed 212.05 KB (0%)
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed 224.52 KB (0%)
@sentry/nextjs (client) 35.14 KB (0%)
@sentry/sveltekit (client) 33.39 KB (0%)
@sentry/node 115.24 KB (0%)
@sentry/aws-serverless 103.72 KB (0%)

@@ -252,7 +252,7 @@ export const buildFeedbackIntegration = ({

const _createActor = (optionOverrides: OverrideFeedbackConfiguration = {}): ActorComponent => {
const shadow = _createShadow(_options);
const actor = Actor({ triggerLabel: _options.triggerLabel, shadow });
const actor = Actor({ triggerLabel: optionOverrides.triggerLabel || _options.triggerLabel, shadow });
Copy link
Member

Choose a reason for hiding this comment

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

I would split and reorder mergedOptions below so that we have:

  1. merged options of _options and optionOverrides
  2. pass a new object of merged options + the actor callbacks to _attachTo

Then we can use the merged options for creating a shadow + actor so that we can avoid this type of issue in the future where the wrong options are used.

@c298lee c298lee requested a review from billyvg May 31, 2024 14:44
@c298lee c298lee enabled auto-merge (squash) May 31, 2024 14:50
@c298lee c298lee merged commit 9865472 into develop May 31, 2024
104 checks passed
@c298lee c298lee deleted the fix-triggerlabel branch May 31, 2024 14:56
c298lee added a commit that referenced this pull request Jun 4, 2024
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.

createWidget and feedbackIntegration missbehavior with triggerLabel?!
2 participants