Skip to content

Conversation

@vuluongj20
Copy link
Contributor

@vuluongj20 vuluongj20 commented Oct 15, 2021

Orange is no longer an accent color in the official design system. It should be replaced with Pink or Yellow, depending on the context. Note: this does not include the color orange in chartPalette -- that's a different palette where orange is still useful for differentiation.

See comments below for before/after screenshots.

Orange is no longer an accent color in the official design system. It should be replaced with Pink or Yellow, depending on the context.
@vuluongj20 vuluongj20 requested a review from a team as a code owner October 15, 2021 20:07
@vuluongj20 vuluongj20 requested a review from a team October 15, 2021 20:07
borderColor: theme.backgroundSecondary,
emphasis: {
areaColor: theme.orange300,
areaColor: theme.pink300,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

worldMapChart--before

worldMapChart

Copy link
Member

Choose a reason for hiding this comment

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

This looks good to me. 👍

function getLevelColor({level = '', theme}) {
const COLORS = {
error: theme.orange400,
error: theme.pink300,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

errorLevel--before

errorLevel

error: theme.pink300,
info: theme.blue300,
warning: theme.orange300,
warning: theme.yellow300,
Copy link
Contributor Author

@vuluongj20 vuluongj20 Oct 15, 2021

Choose a reason for hiding this comment

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

errorLevelWarning--before

errorLevelWarning

Yellow means "warning". This is more inline with the error level indicator in the Issues Index page, which uses yellow for "warning".

@github-actions
Copy link
Contributor

github-actions bot commented Oct 15, 2021

size-limit report

Path Base Size (f87c691) Current Size Change
src/sentry/static/sentry/dist/entrypoints/app.js 52.75 KB 52.73 KB -0.04% 🔽
src/sentry/static/sentry/dist/entrypoints/sentry.css 70.89 KB 70.89 KB 0%

};

return `background-color: ${COLORS[level] || theme.orange400};`;
return `background-color: ${COLORS[level] || theme.pink300};`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Default status is error, which has been updated to Pink 300.

case BreadcrumbType.WARNING:
return {
color: 'orange400',
color: 'yellow300',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

breadcrumbWarning--before

breadcrumbWarning

case BreadcrumbType.SESSION:
return {
color: 'orange300',
color: 'pink300',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

breadcrumbSession--before

breadcrumbSession

border-color: ${p => p.theme.orange500};
color: ${p => p.theme.orange500};
border-color: ${p => p.theme.pink200};
color: ${p => p.theme.pink300};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

stackTraceLine--before

stackTraceLine

border-color: ${p => p.theme.orange500};
color: ${p => p.theme.orange500};
border-color: ${p => p.theme.pink200};
color: ${p => p.theme.pink300};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

stackTraceLine--before

stackTraceLine

path: theme.purple300,
tag: theme.blue300,
codeowners: theme.orange300,
codeowners: theme.pink300,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

suggestedOwners--before

suggestedOwners

font-size: ${p => p.theme.fontSizeMedium};
font-weight: bold;
color: ${p => p.theme.orange400};
color: ${p => p.theme.pink300};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

onboardingInProgressIndicator--before

onboardingInProgressIndicator

flex-grow: 1;
font-size: ${p => p.theme.fontSizeMedium};
color: ${p => p.theme.orange400};
color: ${p => p.theme.pink300};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

onboardingEventWaitingIndicator--before

onboardingEventWaitingIndicator

})}
>
<IconLock color="orange400" />
<IconLock color="pink300" />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

onboardingIncompleteMarker--before

onboardingIncompleteMarker

SkippedIndicator.defaultProps = {
isCircled: true,
color: 'orange400',
color: 'pink300',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

onboardingSkippedIndicator--before

onboardingSkippedIndicator

border-right: none;
margin: -1px 0;
color: ${p => p.theme.orange400};
color: ${p => p.theme.pink300};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

searchOperator--before

searchOperator

position: absolute;
top: -5px;
color: ${p => p.theme.orange400};
color: ${p => p.theme.pink300};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

searchLogicGroup--before

searchLogicGroup

top: -46px;
left: -46px;
border: 4px solid ${p.theme.orange300};
border: 4px solid ${p.theme.pink200};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pulsingIndicator--before

pulsingIndicator

alpha: {
background: `linear-gradient(90deg, ${colors.pink300}, ${colors.yellow300})`,
indicatorColor: colors.orange400,
indicatorColor: colors.pink300,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen Shot 2021-10-15 at 1 19 11 PM

badge

background: colors.orange100,
iconColor: colors.orange400,
background: colors.pink100,
iconColor: colors.pink300,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen Shot 2021-10-15 at 1 20 07 PM

tag

[INSTALLED]: 'success',
[NOT_INSTALLED]: 'gray300',
[PENDING]: 'orange300',
[PENDING]: 'pink300',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

integrationInstallationStatus--before

integrationInstallationStatus

const STAT_OPS = {
'browser-extensions': {title: t('Browser Extension'), color: theme.gray200},
cors: {title: 'CORS', color: theme.orange400},
cors: {title: 'CORS', color: theme.yellow300},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The CORS quantity in the graphs below.

inboundProjectsFilter--before

inboundProjectsFilter

<HomePanelHeader>
<ExternalHomeLink isCentered href={LINKS.DOCUMENTATION}>
<HomeIcon color="orange400">
<HomeIcon color="pink300">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

settingsDocsPanel--before

settingsDocsPanel

@vuluongj20 vuluongj20 changed the title fix(ui): Remove orange from theme fix(ui): Remove uses of Orange Oct 15, 2021
Copy link
Member

@priscilawebdev priscilawebdev left a comment

Choose a reason for hiding this comment

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

left one suggestion, but it looks good to me

green: theme.green300,
yellowDim: theme.yellow300,
yellow: theme.orange300,
yellowDim: theme.pink200,
Copy link
Member

Choose a reason for hiding this comment

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

maybe we should add a todo here? I think it would be nice if we update the names on the backend too

Hardcoding the value of orange400 into the "error" issue level. We will still remove orange from the theme object and everywhere else in the app. This is necessary because we have used orange400 to convey this error level since almost the beginning of Sentry, so changing it to a different color could confuse existing users.
Fixing lint error: "Type 'undefined' cannot be used as an index type". Since `p.level` is potentially undefined, we should use a ternary operator to provide a default cause when it is and avoid the error.
@vuluongj20 vuluongj20 merged commit 7d91819 into master Oct 28, 2021
@vuluongj20 vuluongj20 deleted the vuluong/remove-orange-color branch October 28, 2021 23:34
@github-actions github-actions bot locked and limited conversation to collaborators Nov 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants