-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
fix(feedback): Improve CSS theme variable names and layout #11964
Conversation
size-limit report 📦
|
--font-family: system-ui, 'Helvetica Neue', Arial, sans-serif; | ||
--font-size: 14px; | ||
--z-index: 100000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we removing this as available customization options from constructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ya, it wasn't on docs.sentry.io, and font stuff wasn't possible to override. So i pulled it out, but it's easy to add back.
I think i'd like a general-theme section in there that's neither dark or light mode. In my testing i screwed up a couple times by setting something like themeLight: { foreground: 'red' }
then rendering in dark mode i was confused why i couldn't see my color change. It wouldn't be great to have to specify font in both places, or only in one place and have it copied around imo.
So i was thinking, shorter term at least, we lean into css variables more in the docs, so that's the first stop for people. And that's where everything is available. https://github.com/getsentry/sentry-docs/pull/9961/files
we're missing the outline focus changes :( |
This updates the CSS theme, variable names and also how the css works. The widget is better able to handle cases like: - short desktop window (scrolls form) - phone in portrait (full width if the screen is 600px or less) - phone in landscape (scrolls form) - when screenshots are in use, stick to screen edges I also made it, I think, easier to position the button (aka trigger, aka actor) around the screen. You can override in css `--page-margin: 16px` to dictate the distance from the edge, and override `--inset: auto 0 0 auto;` to position the button in the different corners of the screen. For example, you can put the trigger button on the edge of the screen with something like: ```css #sentry-feedback { --actor-inset: auto 0 calc(50% - var(--header-height)) auto; } ``` Docs are updated in getsentry/sentry-docs#9961 to reflect the changed config/variable names too. Some samples of different situations: | Desc | Img | | --- | --- | | Normal | ![normal](https://github.com/getsentry/sentry-javascript/assets/187460/9223cf1f-9b4b-4bdc-b35a-65ccded71540) | Short Window | ![short window](https://github.com/getsentry/sentry-javascript/assets/187460/5749a07a-ccb6-4c3c-909e-712cf5172f11) | Screenshot editor | ![screenshot](https://github.com/getsentry/sentry-javascript/assets/187460/3a3bbbd0-c982-4d23-b0cf-d6f96ad2a7d1) | Mobile landscape | ![mobile landscape](https://github.com/getsentry/sentry-javascript/assets/187460/aee7ca1f-c6a1-4555-9b07-0ff75aad93e2) | Mobile portrait | ![mobile portrait](https://github.com/getsentry/sentry-javascript/assets/187460/c7c08261-8343-4498-9084-5432c6f1c60e) | Override Color | ![colors override](https://github.com/getsentry/sentry-javascript/assets/187460/99d2ae92-58a0-49f0-982c-95b3e312a694)
This updates the CSS theme, variable names and also how the css works.
The widget is better able to handle cases like:
I also made it, I think, easier to position the button (aka trigger, aka actor) around the screen. You can override in css
--page-margin: 16px
to dictate the distance from the edge, and override--inset: auto 0 0 auto;
to position the button in the different corners of the screen. For example, you can put the trigger button on the edge of the screen with something like:Docs are updated in getsentry/sentry-docs#9961 to reflect the changed config/variable names too.
Some samples of different situations: