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.

Sign up for GitHub

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

Jump to bottom

fix(feedback): Improve CSS theme variable names and layout #11964

Merged
merged 5 commits into from
May 9, 2024
Merged

fix(feedback): Improve CSS theme variable names and layout #11964

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a476d4f
fix(feedback): Improve CSS theme variable names and layout
ryan953 May 9, 2024
43b66f2
fix border in dark mode
ryan953 May 9, 2024
ff2f6f5
be consistent about setting color and fill for icons
ryan953 May 9, 2024
d8cf4aa
Apply suggestions from code review
ryan953 May 9, 2024
4fdfdf8
fix some types
ryan953 May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 37 additions & 36 deletions packages/feedback/src/constants/theme.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,57 +1,58 @@
const LIGHT_BACKGROUND = '#ffffff';
const INHERIT = 'inherit';
const SUBMIT_COLOR = 'rgba(108, 95, 199, 1)';
const PURPLE = 'rgba(88, 74, 192, 1)';
const PURPLE_HOVER = 'rgba(108, 95, 199, 1)';

export const LIGHT_THEME = {
fontFamily: "system-ui, 'Helvetica Neue', Arial, sans-serif",
fontSize: '14px',

foreground: '#2b2233',
background: LIGHT_BACKGROUND,
success: '#268d75',
error: '#df3338',

zIndex: 100000,
successForeground: '#268d75',
errorForeground: '#df3338',
background: '#ffffff',
border: '1.5px solid rgba(41, 35, 47, 0.13)',
boxShadow: '0px 4px 24px 0px rgba(43, 34, 51, 0.12)',

backgroundHover: '#f6f6f7',
borderRadius: '25px',

formBorderRadius: '20px',
formContentBorderRadius: '6px',

submitForeground: LIGHT_BACKGROUND,
submitBackground: 'rgba(88, 74, 192, 1)',
submitForegroundHover: LIGHT_BACKGROUND,
submitBackgroundHover: SUBMIT_COLOR,
submitBorder: SUBMIT_COLOR,
inputForeground: INHERIT,
inputBackground: INHERIT,
inputBackgroundHover: INHERIT,
inputBackgroundFocus: INHERIT,
inputBorder: 'var(--border)',
inputBorderRadius: '6px',
inputOutlineFocus: PURPLE_HOVER,

buttonForeground: INHERIT,
buttonForegroundHover: INHERIT,
buttonBackground: 'var(--background)',
buttonBackgroundHover: '#f6f6f7',
buttonBorder: 'var(--border)',
buttonOutlineFocus: 'var(--input-outline-focus)',

submitForeground: '#ffffff',
submitForegroundHover: '#ffffff',
submitBackground: PURPLE,
submitBackgroundHover: PURPLE_HOVER,
submitBorder: PURPLE_HOVER,
submitBorderRadius: 'var(--button-border-radius)',
submitOutlineFocus: '#29232f',

cancelForeground: 'var(--foreground)',
cancelBackground: 'transparent',
cancelForegroundHover: 'var(--foreground)',
cancelBackgroundHover: 'var(--background-hover)',
cancelBorder: 'var(--border)',
cancelOutlineFocus: 'var(--input-outline-focus)',
triggerBackground: 'var(--background)',
triggerBackgroundHover: 'var(--button-background-hover)',
triggerBorderRadius: '1.7em/50%',

inputBackground: INHERIT,
inputForeground: INHERIT,
inputBorder: 'var(--border)',
inputOutlineFocus: SUBMIT_COLOR,
dialogBackground: 'var(--background)',
dialogBorderRadius: '20px',
};

export const DEFAULT_THEME = {
light: LIGHT_THEME,
dark: {
...LIGHT_THEME,

background: '#29232f',
backgroundHover: '#352f3b',
foreground: '#ebe6ef',
border: '1.5px solid rgba(235, 230, 239, 0.15)',
successForeground: '#2da98c',
errorForeground: '#f55459',
background: '#29232f',
border: '1.5px solid rgba(235, 230, 239, 0.5)',
boxShadow: '0px 4px 24px 0px rgba(43, 34, 51, 0.12)',

success: '#2da98c',
error: '#f55459',
buttonBackgroundHover: '#352f3b',
},
};
15 changes: 8 additions & 7 deletions packages/feedback/src/core/components/Actor.css.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export function createActorStyles(): HTMLStyleElement {
.widget__actor {
position: fixed;
z-index: var(--z-index);
margin: 0;
margin: var(--page-margin);
inset: var(--actor-inset);

display: flex;
Expand All @@ -20,14 +20,15 @@ export function createActorStyles(): HTMLStyleElement {
font-family: inherit;
font-size: var(--font-size);
font-weight: 600;
line-height: 16px;
line-height: 1.14em;
text-decoration: none;

background-color: var(--background);
border-radius: var(--border-radius);
background-color: var(--trigger-background);
border-radius: var(--trigger-border-radius);
border: var(--border);
box-shadow: var(--box-shadow);
color: var(--foreground);
fill: var(--foreground);
cursor: pointer;
opacity: 1;
transition: transform 0.2s ease-in-out;
Expand All @@ -41,12 +42,12 @@ export function createActorStyles(): HTMLStyleElement {
}

.widget__actor:hover {
background-color: var(--background-hover);
background-color: var(--trigger-background-hover);
}

.widget__actor svg {
width: 16px;
height: 16px;
width: 1.14em;
height: 1.14em;
}

@media (max-width: 600px) {
Expand Down
42 changes: 21 additions & 21 deletions packages/feedback/src/core/createMainStyles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,23 @@ import { DOCUMENT } from '../constants';

function getThemedCssVariables(theme: FeedbackInternalOptions['themeLight']): string {
return `
--background: ${theme.background};
--background-hover: ${theme.backgroundHover};
--foreground: ${theme.foreground};
--error: ${theme.error};
--success: ${theme.success};
--success-foreground: ${theme.successForeground};
--error-foreground: ${theme.errorForeground};
--background: ${theme.background};
--border: ${theme.border};
--border-radius: ${theme.borderRadius};
--box-shadow: ${theme.boxShadow};

--button-foreground: ${theme.buttonForeground};
--button-foreground-hover: ${theme.buttonForegroundHover};
--button-background: ${theme.buttonBackground};
--button-background-hover: ${theme.buttonBackgroundHover};
--button-border: ${theme.buttonBorder};
--button-outline-focus: ${theme.buttonOutlineFocus};

--trigger-background: ${theme.triggerBackground};
--trigger-background-hover: ${theme.triggerBackgroundHover};
--trigger-border-radius: ${theme.triggerBorderRadius};
`;
}

Expand All @@ -21,25 +30,16 @@ export function createMainStyles({ colorScheme, themeDark, themeLight }: Feedbac
const style = DOCUMENT.createElement('style');
style.textContent = `
:host {
--z-index: ${themeLight.zIndex};
--font-family: ${themeLight.fontFamily};
--font-size: ${themeLight.fontSize};

font-family: var(--font-family);
font-size: var(--font-size);
--font-family: system-ui, 'Helvetica Neue', Arial, sans-serif;
--font-size: 14px;
--z-index: 100000;
Comment on lines +33 to +35
Copy link
Member

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?

Copy link
Member Author

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


--page-margin: 16px;
--actor-inset: auto var(--page-margin) var(--page-margin) auto;
--inset: auto 0 0 auto;
--actor-inset: var(--inset);

.brand-link path {
fill: ${colorScheme === 'dark' ? '#fff' : '#362d59'};
}
@media (prefers-color-scheme: dark)
{
path: {
fill: '#fff';
}
}
font-family: var(--font-family);
font-size: var(--font-size);

${getThemedCssVariables(colorScheme === 'dark' ? themeDark : themeLight)}
}
Expand Down
Loading
Loading