Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 4 additions & 3 deletions UNRELEASED.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t

### Enhancements

- Update `VisuallyHidden` styles to not use `top` or `clip` ([#4641](https://github.com/Shopify/polaris-react/pull/4641)).
- Added `PlainAction` type to `ComplexAction`. ([#4489](https://github.com/Shopify/polaris-react/pull/4489)
- Updated `VisuallyHidden` styles to not use `top` or `clip` ([#4641](https://github.com/Shopify/polaris-react/pull/4641)).
- Added `PlainAction` type to `ComplexAction`. ([#4489](https://github.com/Shopify/polaris-react/pull/4489))

### Bug fixes

Expand All @@ -23,6 +23,7 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t

### Code quality

Clean up Button styling and $button-filled mixin([#4635](https://github.com/Shopify/polaris-react/pull/4635))
- Cleaned up Button styling and $button-filled mixin([#4635](https://github.com/Shopify/polaris-react/pull/4635))
- Removed miscellaneous css custom properties ([#4620](https://github.com/Shopify/polaris-react/pull/4620))

### Deprecations
8 changes: 0 additions & 8 deletions documentation/Color system.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,14 +268,8 @@ Used to decorate elements where color does convey a specific meaning in componen
| `--p-button-drop-shadow` | `0 1px 0 rgba(0, 0, 0, 0.05)` |
| `--p-button-inner-shadow` | `inset 0 -1px 0 rgba(0, 0, 0, 0.2)` |
| `--p-button-pressed-inner-shadow` | `inset 0 1px 0 rgba(0, 0, 0, 0.15)` |
| `--p-override-none` | `none` |
| `--p-override-transparent` | `transparent` |
| `--p-override-one` | `1` |
| `--p-override-visible` | `visible` |
| `--p-override-zero` | `0` |
| `--p-override-loading-z-index` | `514` |
| `--p-button-font-weight` | `500` |
| `--p-non-null-content` | `''` |
| `--p-choice-size` | `2rem` |
| `--p-icon-size` | `1rem` |
| `--p-choice-margin` | `0.1rem` |
Expand All @@ -285,7 +279,6 @@ Used to decorate elements where color does convey a specific meaning in componen
| `--p-banner-border-highlight` | `inset 0 0.1rem 0 0 var(--p-border-highlight-subdued), inset 0 0 0 0.1rem var(--p-border-highlight-subdued)` |
| `--p-banner-border-warning` | `inset 0 0.1rem 0 0 var(--p-border-warning-subdued), inset 0 0 0 0.1rem var(--p-border-warning-subdued)` |
| `--p-banner-border-critical` | `inset 0 0.1rem 0 0 var(--p-border-critical-subdued), inset 0 0 0 0.1rem var(--p-border-critical-subdued)` |
| `--p-badge-mix-blend-mode` | `luminosity` |
| `--p-thin-border-subdued` | `0.1rem solid var(--p-border-subdued)` |
| `--p-text-field-spinner-offset` | `0.2rem` |
| `--p-text-field-focus-ring-offset` | `-0.4rem` |
Expand All @@ -297,5 +290,4 @@ Used to decorate elements where color does convey a specific meaning in componen
| `--p-ease` | `cubic-bezier(0.4, 0.22, 0.28, 1)` |
| `--p-range-slider-thumb-size-base` | `1.6rem` |
| `--p-range-slider-thumb-size-active` | `2.4rem` |
| `--p-range-slider-thumb-scale` | `1.5` |
| `--p-badge-font-weight` | `400` |
1 change: 0 additions & 1 deletion src/components/Badge/Badge.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ $pip-spacing: ($height - $pip-size) / 2;
align-items: center;
padding: $vertical-padding $horizontal-padding;
background-color: var(--p-surface-neutral);
border: var(--p-override-zero);
border-radius: $height;
font-size: rem(13px);
line-height: $extra-small-height;
Expand Down
1 change: 0 additions & 1 deletion src/components/Banner/Banner.scss
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,6 @@ $spinner-size: rem(20px);
}

.Heading {
padding-top: var(--p-override-none);
word-break: break-word;
}

Expand Down
24 changes: 2 additions & 22 deletions src/components/Breadcrumbs/Breadcrumbs.scss
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ $icon-size: rem(20px);
position: relative;
display: flex;
align-items: center;
justify-content: center;
min-height: control-height();
min-width: control-height();
color: var(--p-text-subdued);
text-decoration: none;
margin: 0;
Expand All @@ -24,10 +26,6 @@ $icon-size: rem(20px);

&:active {
background-color: var(--p-surface-pressed);

.ContentWrapper {
background: var(--p-override-transparent);
}
Comment on lines -27 to -30
Copy link
Member

@alex-page alex-page Nov 12, 2021

Choose a reason for hiding this comment

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

Do we need the HTML element with .ContentWrapper classname? Or can we flatten this components HTML?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think we need it—I can consolidate its relevant CSS into .Breadcrumb and replace its HTML with a shorthand fragment so that contentMarkup still has one parent element (which you can check out in this commit).

Copy link
Contributor Author

@lgriffee lgriffee Nov 15, 2021

Choose a reason for hiding this comment

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

Also along the line of consolidation I just want to note that there might be more rollback values that need to be removed in these files. Not sure if there was work done already to remove them so it’s definitely possible that they are needed overrides. I didn’t look into them in this PR to keep a narrow scope. But if it’s something important to address now or in the future I can create a ticket to look into it.

}

&:hover,
Expand All @@ -42,9 +40,6 @@ $icon-size: rem(20px);
// stylelint-disable selector-max-specificity
&:focus {
outline: none;
.ContentWrapper {
background: var(--p-override-transparent);
}
}

&:focus:not(:active) {
Expand All @@ -53,21 +48,6 @@ $icon-size: rem(20px);
// stylelint-enable selector-max-specificity
}

.ContentWrapper {
position: relative;
display: flex;
align-items: center;
justify-content: center;
padding: 0;
margin-left: 0;
background: transparent;
height: control-slim-height();
width: control-slim-height();
border-radius: var(--p-border-radius-wide);
will-change: background;
transition: background duration() easing();
}

.Content {
@include truncate;
position: relative;
Expand Down
4 changes: 2 additions & 2 deletions src/components/Breadcrumbs/Breadcrumbs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ export function Breadcrumbs({breadcrumbs}: BreadcrumbsProps) {
const {content} = breadcrumb;

const contentMarkup = (
<span className={styles.ContentWrapper}>
<>
<span className={styles.Icon}>
<Icon source={ArrowLeftMinor} />
</span>
<VisuallyHidden>{content}</VisuallyHidden>
</span>
</>
);

const breadcrumbMarkup =
Expand Down
1 change: 0 additions & 1 deletion src/components/Choice/Choice.scss
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ $control-vertical-adjustment: rem(2px);
align-items: stretch;
width: var(--p-choice-size);
height: var(--p-choice-size);
margin-top: var(--p-override-none);
margin-right: spacing(tight);

> * {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,6 @@ $stacking-order: (
}

.MoreFiltersButtonContainer {
> div {
margin-left: var(--p-override-none);
}

.Item > div > button {
white-space: nowrap;
border-top-left-radius: 0;
Expand Down
4 changes: 0 additions & 4 deletions src/components/FooterHelp/FooterHelp.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,11 @@ $border-radius: 999px;
display: inline-flex;
align-items: center;
padding: spacing(loose) spacing(loose) spacing(loose) spacing();
border-top: var(--p-override-none);
border-bottom: var(--p-override-none);
width: 100%;
justify-content: center;

@include page-content-when-not-fully-condensed {
width: auto;
border: var(--p-override-none);
border-radius: var(--p-override-none);
}
}

Expand Down
1 change: 0 additions & 1 deletion src/components/Frame/Frame.scss
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ $skip-vertical-offset: rem(10px);
}

.hasTopBar & {
top: var(--p-override-zero);
z-index: var(
--p-override-loading-z-index,
z-index(loading-bar, $fixed-element-stacking-order)
Expand Down
1 change: 0 additions & 1 deletion src/components/Frame/components/Toast/Toast.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ $Backdrop-opacity: 0.88;
padding: spacing(tight) spacing();
border-radius: var(--p-border-radius-wide);
background: var(--p-surface);
box-shadow: var(--p-override-none);
color: var(--p-text);
margin-bottom: spacing(loose);

Expand Down
5 changes: 0 additions & 5 deletions src/components/Link/Link.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,6 @@
text-decoration: none;
}

&:focus {
$chrome-default-outline: rgba(0, 103, 244, 0.247) auto rem(4.5px);
outline: var(--p-override-none);
}

&:focus:not(:active) {
outline: var(--p-focused) auto 1px;
}
Expand Down
4 changes: 0 additions & 4 deletions src/components/Modal/components/CloseButton/CloseButton.scss
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@
@include recolor-icon(var(--p-icon-hovered));
}

&:focus {
background: var(--p-override-transparent);
}

&:active {
background: var(--p-surface-pressed);
}
Expand Down
7 changes: 2 additions & 5 deletions src/components/Navigation/Navigation.scss
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ $disabled-fade: 0.6;
&:active {
@include recolor-icon(
var(--p-icon-hovered),
var(--p-override-transparent),
transparent,
$filter-color: filter('action')
);
background: var(--p-background-hovered);
Expand Down Expand Up @@ -375,10 +375,7 @@ $secondary-item-font-size: rem(15px);
&:hover,
&:focus {
background: var(--p-background-hovered);
@include recolor-icon(
var(--p-icon-hovered),
var(--p-override-transparent)
);
@include recolor-icon(var(--p-icon-hovered), transparent);

@media (-ms-high-contrast: active) {
@include recolor-icon(
Expand Down
2 changes: 1 addition & 1 deletion src/components/README.md

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ $range-thumb-border-error: rem(2px) solid color('red');
height: $range-track-height;
border-radius: $range-thumb-size;

--unselected-range: #{var(--p-override-transparent)};
--unselected-range: transparent;
--selected-range: #{var(--p-interactive)};
--gradient-colors: var(--unselected-range) 0%,
var(--unselected-range) var(--Polaris-RangeSlider-progress-lower),
Expand Down Expand Up @@ -91,7 +91,7 @@ $range-thumb-border-error: rem(2px) solid color('red');
}

&:active {
transform: scale(var(--p-range-slider-thumb-scale));
transform: scale(1.5);
}

&:focus {
Expand Down Expand Up @@ -133,11 +133,6 @@ $range-output-spacing: rem(16px);
transition-timing-function: var(--p-ease);
transform: translateX(calc(-50% + #{$range-thumb-size / 2}));

.Thumbs:hover + &,
.Thumbs:focus + & {
opacity: var(--p-override-zero);
}

.Thumbs:active + & {
$range-thumb-size-difference: var(--p-range-slider-thumb-size-active) -
var(--p-range-slider-thumb-size-base);
Expand Down
13 changes: 2 additions & 11 deletions src/components/RangeSlider/components/SingleThumb/SingleThumb.scss
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ $stacking-order: (

.Input {
--progress-lower: #{var(--p-interactive)};
--progress-upper: #{var(--p-override-transparent)};
--progress-upper: transparent;
// create-react-app v2 leverages postcss-preset-env to transpile modern CSS
// into something kinda supported by older browsers. Unfortunately its
// custom properties transpiler has a bug that means it doesn't like multiple
Expand Down Expand Up @@ -120,7 +120,7 @@ $stacking-order: (

&:active {
@include range-thumb-selectors {
transform: scale(var(--p-range-slider-thumb-scale));
transform: scale(1.5);
}
}

Expand All @@ -136,10 +136,6 @@ $stacking-order: (
.error & {
--progress-lower: #{var(--p-action-critical)};

@include range-track-selectors {
background-color: var(--p-override-none);
}

@include range-thumb-selectors {
border-color: var(--p-action-critical);
background: var(--p-action-critical);
Expand Down Expand Up @@ -182,11 +178,6 @@ $range-output-spacing: rem(16px);
transition-duration: duration();
transition-timing-function: easing();

.Input:hover + &,
.Input:focus + & {
opacity: var(--p-override-zero);
}

.Input:active + & {
$range-thumb-size-difference: var(--p-range-slider-thumb-size-active) -
var(--p-range-slider-thumb-size-base);
Expand Down
15 changes: 0 additions & 15 deletions src/components/Select/Select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -41,21 +41,6 @@ $stacking-order: (
}
}

.placeholder {
// stylelint-disable-next-line selector-max-class
&.error .Input {
// This is the only place this color is used.
// stylelint-disable-next-line color-no-hex
color: #9c9798;
}

// stylelint-disable-next-line selector-max-class, selector-max-specificity
&.error .Input:-moz-focusring {
color: transparent;
text-shadow: var(--p-override-none);
}
}
Comment on lines -44 to -57
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing unused class. Here's the legacy commit from when it was added

Copy link
Member

Choose a reason for hiding this comment

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

Amazing! Thanks for the context Laura. I think it makes sense to remove this code.


.Content {
@include text-style-input;
position: relative;
Expand Down
1 change: 0 additions & 1 deletion src/components/ThemeProvider/tests/ThemeProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ describe('<ThemeProvider />', () => {

expect(themeProvider.find('div')).toHaveReactProps({
style: expect.objectContaining({
'--p-override-zero': expect.any(String),
'--p-background': expect.any(String),
'--p-text': expect.any(String),
'--p-interactive': expect.any(String),
Expand Down
3 changes: 0 additions & 3 deletions src/components/TopBar/TopBar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,6 @@ $context-control-expand-after: 1400px;
fill: var(--p-icon); // Icon will inherit this fill
transition: (duration() - 33ms) fill easing() 33ms;

&.focused {
background-color: var(--p-override-transparent);
}
&.focused:active {
background-color: var(--p-surface-pressed);
}
Expand Down
5 changes: 1 addition & 4 deletions src/components/TopBar/components/Menu/Menu.scss
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ $activator-variables: (

&:focus {
@include focus-ring($style: 'focused');
background-color: var(
--top-bar-background-lighter,
var(--p-override-transparent)
);
background-color: var(--top-bar-background-lighter);
outline: none;
}

Expand Down
8 changes: 0 additions & 8 deletions src/utilities/theme/tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,8 @@ export const Tokens = {
buttonPressedInnerShadow: 'inset 0 1px 0 rgba(0, 0, 0, 0.15)',

// Overrides
overrideNone: 'none',
overrideTransparent: 'transparent',
overrideOne: '1',
overrideVisible: 'visible',
overrideZero: '0',
overrideLoadingZIndex: '514',
buttonFontWeight: '500',
nonNullContent: "''",
choiceSize: rem('20px'),
iconSize: rem('10px'),
choiceMargin: rem('1px'),
Expand All @@ -35,7 +29,6 @@ export const Tokens = {
bannerBorderHighlight: buildBannerBorder('--p-border-highlight-subdued'),
bannerBorderWarning: buildBannerBorder('--p-border-warning-subdued'),
bannerBorderCritical: buildBannerBorder('--p-border-critical-subdued'),
badgeMixBlendMode: 'luminosity',
thinBorderSubdued: `${rem('1px')} solid var(--p-border-subdued)`,
textFieldSpinnerOffset: rem('2px'),
textFieldFocusRingOffset: rem('-4px'),
Expand All @@ -47,7 +40,6 @@ export const Tokens = {
ease: 'cubic-bezier(0.4, 0.22, 0.28, 1)',
rangeSliderThumbSizeBase: rem('16px'),
rangeSliderThumbSizeActive: rem('24px'),
rangeSliderThumbScale: '1.5',
badgeFontWeight: '400',
};

Expand Down