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

Dark Theme Fixes #10416

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
2 changes: 1 addition & 1 deletion react-common/styles/controls/EditorToggle.less
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@

& > .common-button {
background: none;
color: var(--pxt-neutral-background1);
color: var(--pxt-neutral-foreground1);
transition: color .25s;
margin: 0;
width: 100%;
Expand Down
2 changes: 1 addition & 1 deletion react-common/styles/extensions/ExtensionCard.less
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
border-top: solid 1px var(--pxt-neutral-stencil1);
flex-shrink: 0;
font-size: 16px;
color: var(--pxt-secondary-background);
color: var(--pxt-neutral-foreground1);

overflow: hidden;
border-bottom-left-radius: 0.5rem;
Expand Down
1 change: 1 addition & 0 deletions react-common/styles/theming/base-theme.less
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
--pxt-neutral-alpha10: rgba(0, 0, 0, 0.1);
--pxt-neutral-alpha20: rgba(0, 0, 0, 0.2);
--pxt-neutral-alpha50: rgba(0, 0, 0, 0.5);
--pxt-neutral-alpha80: rgba(0, 0, 0, 0.8);

--pxt-link: #3977B4;
--pxt-link-hover: #204467;
Expand Down
1 change: 1 addition & 0 deletions theme/color-themes/high-contrast.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@
"pxt-neutral-alpha10": "rgba(255, 255, 255, 0.1)",
"pxt-neutral-alpha20": "rgba(255, 255, 255, 0.2)",
"pxt-neutral-alpha50": "rgba(255, 255, 255, 0.5)",
"pxt-neutral-alpha80": "rgba(255, 255, 255, 0.8)",

"pxt-link": "#807FFF",
"pxt-link-hover": "#1b19ff",
Expand Down
2 changes: 1 addition & 1 deletion theme/semantic-ui-overrides.less
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ body.pxt-theme-root {
}

.description {
color: var(--pxt-neutral-alpha50);
color: var(--pxt-neutral-alpha80);
}
}

Expand Down
4 changes: 2 additions & 2 deletions webapp/src/components/assetEditor/assetGallery.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ class AssetGalleryImpl extends React.Component<AssetGalleryProps, AssetGallerySt
<AssetTopbar />
<div className={`asset-editor-card-list ${view !== GalleryView.User ? "hidden" : ""}`}>
<AssetCardList assets={filterAssets(userAssets, isBlocksProject)}>
<Button className="create-new inverted"
<Button className="create-new"
Copy link
Contributor

Choose a reason for hiding this comment

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

If I'm understanding your commit comment correctly, this fix is supposed to address microsoft/pxt-arcade#6780. I'm not sure if this fixes the problem in the regular dark theme, but it's still broken in high contrast on the upload target. Sorry if this is unrelated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a different bug. This is actually compensating for the fix for microsoft/pxt-arcade#6775. That bug required an override in arcade-dark css for inverted buttons (see microsoft/pxt-arcade#6785), but that override was also affecting these buttons, which was undesirable.

Ultimately, the inverted class here is unnecessary because the colors are set directly for them, anyway. So the simplest fix is just to remove the inverted class from these elements altogether.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, interesting! Thanks for the clarification!

disabled={disableCreateButton}
leftIcon="icon huge add circle"
title={lf("Create a new asset")}
Expand All @@ -142,7 +142,7 @@ class AssetGalleryImpl extends React.Component<AssetGalleryProps, AssetGallerySt
this.assetCreateOptions.map((opt, i) => {
return <Button
key={i}
className="asset-editor-create-button inverted"
className="asset-editor-create-button"
leftIcon={`icon ${opt.icon}`}
label={opt.label}
title={lf("Create a new {0} asset", opt.label)}
Expand Down