Skip to content
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

feat(feedback): Configure feedback border radius #10289

Merged
merged 3 commits into from
Jan 23, 2024

Conversation

c298lee
Copy link
Member

@c298lee c298lee commented Jan 22, 2024

Adds ability to configure all border radiuses on feedback widget

Closes #10256

@c298lee c298lee requested a review from a team January 22, 2024 16:55
Copy link
Contributor

github-actions bot commented Jan 22, 2024

size-limit report 📦

Path Size
@sentry/browser (incl. Tracing, Replay, Feedback) - Webpack (gzipped) 77.77 KB (+0.11% 🔺)
@sentry/browser (incl. Tracing, Replay) - Webpack (gzipped) 68.95 KB (0%)
@sentry/browser (incl. Tracing, Replay with Canvas) - Webpack (gzipped) 72.84 KB (0%)
@sentry/browser (incl. Tracing, Replay) - Webpack with treeshaking flags (gzipped) 62.58 KB (0%)
@sentry/browser (incl. Tracing) - Webpack (gzipped) 32.96 KB (0%)
@sentry/browser (incl. Feedback) - Webpack (gzipped) 31.24 KB (+0.2% 🔺)
@sentry/browser (incl. sendFeedback) - Webpack (gzipped) 31.25 KB (+0.21% 🔺)
@sentry/browser - Webpack (gzipped) 22.53 KB (0%)
@sentry/browser (incl. Tracing, Replay, Feedback) - ES6 CDN Bundle (gzipped) 75.34 KB (+0.1% 🔺)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (gzipped) 66.9 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (gzipped) 32.73 KB (0%)
@sentry/browser - ES6 CDN Bundle (gzipped) 24.25 KB (0%)
@sentry/browser (incl. Tracing, Replay) - ES6 CDN Bundle (minified & uncompressed) 210.77 KB (0%)
@sentry/browser (incl. Tracing) - ES6 CDN Bundle (minified & uncompressed) 98.75 KB (0%)
@sentry/browser - ES6 CDN Bundle (minified & uncompressed) 72.58 KB (0%)
@sentry/browser (incl. Tracing) - ES5 CDN Bundle (gzipped) 35.81 KB (0%)
@sentry/react (incl. Tracing, Replay) - Webpack (gzipped) 69.35 KB (0%)
@sentry/react - Webpack (gzipped) 22.58 KB (0%)
@sentry/nextjs Client (incl. Tracing, Replay) - Webpack (gzipped) 85.97 KB (0%)
@sentry/nextjs Client - Webpack (gzipped) 50.25 KB (0%)
@sentry-internal/feedback - Webpack (gzipped) 17.21 KB (+0.41% 🔺)

@@ -38,7 +39,10 @@ const LIGHT_THEME = {
inputBackground: INHERIT,
inputForeground: INHERIT,
inputBorder: 'var(--border)',
inputBorderRadius: '6px',
Copy link
Member

Choose a reason for hiding this comment

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

let's move/rename this to formContentBorderRadius and apply it to inputs and submit/cancel buttons - from a design POV, the children elements inside of the form should have the same border radius

packages/feedback/src/types/index.ts Outdated Show resolved Hide resolved
@c298lee c298lee requested a review from billyvg January 22, 2024 20:41
@c298lee c298lee merged commit a0b987a into develop Jan 23, 2024
85 checks passed
@c298lee c298lee deleted the cl/feedback-border-radius-config branch January 23, 2024 15:43
/**
* Border radius for dialog
*/
formBorderRadius: string;
Copy link
Member

Choose a reason for hiding this comment

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

hmm, is form clear here as a prefix (from a user pov) - meaning is it clear what this border radius is referring to?

Copy link
Member Author

Choose a reason for hiding this comment

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

we use formTitle, and refer to Feedback form in the docs, so this should be clear

Copy link
Member

Choose a reason for hiding this comment

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

/**
* Border radius for form inputs
*/
formContentBorderRadius: string;
Copy link
Member

Choose a reason for hiding this comment

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

not 100% sure but what about maybe inputBorderRadius or something like this?

Copy link
Member Author

Choose a reason for hiding this comment

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

we considered inputBorderRadius, but this applies to the input and the buttons in the form (cancel & submit) so we wanted it to be more clear

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.

[User Feedback] Add border radius config option
3 participants