Skip to content

feat: Remove print SCSS mixins #4995

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

Merged
merged 9 commits into from
Feb 1, 2022
Merged

feat: Remove print SCSS mixins #4995

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ccbc235
feat: Replace with
aaronccasanova Feb 1, 2022
b79a782
feat: Remove mixin
aaronccasanova Feb 1, 2022
1d8e9c2
docs: Update the v9 unreleased file
aaronccasanova Feb 1, 2022
0f2c8ee
Merge branch 'v9.0.0-major' of https://github.com/Shopify/polaris-rea…
aaronccasanova Feb 1, 2022
6a2284c
Merge branch 'v9.0.0-major' of https://github.com/Shopify/polaris-rea…
aaronccasanova Feb 1, 2022
4acfcad
feat: Remove print-hidden in favor of hidden-when-printing
aaronccasanova Feb 1, 2022
ebffa41
feat: Remove hidden-when-printing mixin
aaronccasanova Feb 1, 2022
4f02a1a
docs: Update the v9 unreleased file
aaronccasanova Feb 1, 2022
5c9e015
feat: Remove print mixins import
aaronccasanova Feb 1, 2022
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
1 change: 1 addition & 0 deletions UNRELEASED-v9.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Use [the changelog guidelines](/documentation/Versioning%20and%20changelog.md) t
- Removed the `unstyled-link()` mixin and replaced with values ([#4951](https://github.com/Shopify/polaris-react/pull/4951))
- Removed the `unstyled-list()` mixin and replaced with values ([#4960](https://github.com/Shopify/polaris-react/pull/4960))
- Removed the `high-contrast-outline()` and `high-contrast-border()` mixins and replaced with tokens and values ([#4962](https://github.com/Shopify/polaris-react/pull/4962))
- Removed the `when-printing`, `when-not-printing`, `hidden-when-printing`, and `print-hidden` scss mixins ([#4995](https://github.com/Shopify/polaris-react/pull/4995))
- Replaced the `icon-size()` function with the `--p-icon-size-medium` custom property ([#4990](https://github.com/Shopify/polaris-react/pull/4990))

**Sass global variables**
Expand Down
6 changes: 5 additions & 1 deletion src/components/ActionMenu/ActionMenu.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
@import '../../styles/common';

.ActionMenu {
@include print-hidden;
// Needed for JS calculations
width: 100%;
display: flex;
justify-content: flex-end;

@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
Copy link
Member

@alex-page alex-page Feb 1, 2022

Choose a reason for hiding this comment

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

Now that I see this code in context... I wonder if it even needs the important.

Copy link
Member Author

@aaronccasanova aaronccasanova Feb 1, 2022

Choose a reason for hiding this comment

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

Totally, I imagine a lot of these are unnecessary. My guess is the original hidden-when-printing mixin included !important to cover all edge cases. Happy to spin up a follow up issue to assess all !importants throughout polaris-react.

Copy link
Member

@BPScott BPScott Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I imagine they were needed before because there were linting rules to try and push usage of mixins up to the top of blocks. That meant that (omiting width and other not relevant properties):

.ActionMenu {
 @include print-hidden;
  display: flex;
}

compiled to

@media print {
  .ActionMenu {    
    display: none !important;
  }
}

.ActionMenu {
  display: flex;
}

In which case the !important was uh important, as without it the display:flex would take precidence in print stylesheets.

Now we're inlining we are free of that "mixins gotta go first to keep lint happy" restriction we could write:

.ActionMenu {
  display: flex;

  @media print {
    display: none;
  }
}

which would compile to the following, which would do the right thing with needing !important.

.ActionMenu {
  display: flex;
}

@media print {
  .ActionMenu {
    display: none;
  }
}

}
}
2 changes: 1 addition & 1 deletion src/components/AppProvider/AppProvider.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ body {
// stylelint-disable color-no-hex
background-color: #f6f6f7;

@include when-printing {
@media print {
// AppProvider sets styles on the body. These needs to
// be overridden using !important.
// stylelint-disable-next-line declaration-no-important
Expand Down
5 changes: 4 additions & 1 deletion src/components/Card/Card.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@

.Section-hideOnPrint,
.hideOnPrint {
@include hidden-when-printing;
@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}
}

.Header {
Expand Down
2 changes: 1 addition & 1 deletion src/components/Collapsible/Collapsible.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
}

.expandOnPrint {
@include when-printing {
@media print {
// stylelint-disable-next-line declaration-no-important
max-height: none !important;
overflow: visible;
Expand Down
30 changes: 23 additions & 7 deletions src/components/Frame/Frame.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
display: flex;
background-color: var(--p-background);

@include when-printing {
@media print {
background-color: transparent;
}

Expand All @@ -18,7 +18,6 @@
}

.Navigation {
@include hidden-when-printing;
position: fixed;
z-index: var(--p-z-8);
top: 0;
Expand All @@ -30,6 +29,11 @@
outline: none;
transform: translateX(0%);

@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}

@include frame-when-nav-displayed {
z-index: 1;
left: var(--pc-frame-offset);
Expand Down Expand Up @@ -72,7 +76,6 @@
}

.NavigationDismiss {
@include hidden-when-printing;
@include recolor-icon(var(--p-surface));
@include focus-ring;
position: absolute;
Expand All @@ -91,6 +94,11 @@
cursor: pointer;
transition: opacity var(--p-duration-100) var(--p-ease);

@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}

.Navigation-visible & {
pointer-events: all;
opacity: 1;
Expand All @@ -117,14 +125,18 @@
}

.TopBar {
@include hidden-when-printing;
position: fixed;
z-index: var(--p-z-4);
top: 0;
left: 0;
width: 100%;
height: top-bar-height();

@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}

@include frame-when-nav-displayed {
left: var(--pc-frame-offset);
width: calc(100% - var(--pc-frame-offset));
Expand Down Expand Up @@ -158,7 +170,7 @@
@include frame-when-nav-displayed {
.hasNav & {
padding-left: layout-width(nav);
@include when-printing {
@media print {
padding-left: 0;
}
@include safe-area-for(padding-left, layout-width(nav), left);
Expand All @@ -167,7 +179,7 @@

.hasTopBar & {
padding-top: top-bar-height();
@include when-printing {
@media print {
padding-top: 0;
}
}
Expand Down Expand Up @@ -202,13 +214,17 @@
}

.LoadingBar {
@include hidden-when-printing;
position: fixed;
z-index: var(--p-z-6);
top: 0;
right: 0;
left: 0;

@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}

@include frame-when-nav-displayed {
.hasNav & {
left: var(--pc-frame-offset);
Expand Down
32 changes: 25 additions & 7 deletions src/components/Page/components/Header/Header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ $action-menu-rollup-computed-width: 40px;
padding-right: $action-menu-rollup-computed-width;
}

@include print-hidden;
@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}
}

.BreadcrumbWrapper {
Expand All @@ -49,7 +52,10 @@ $action-menu-rollup-computed-width: 40px;
max-width: 100%;
margin-right: var(--p-space-4);

@include print-hidden;
@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}
}

.PaginationWrapper {
Expand All @@ -72,15 +78,21 @@ $action-menu-rollup-computed-width: 40px;
}
}

@include print-hidden;
@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}
}

.AdditionalNavigationWrapper {
display: flex;
flex: 1 0 auto;
justify-content: flex-end;

@include print-hidden;
@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}
}

///
Expand Down Expand Up @@ -115,7 +127,10 @@ $action-menu-rollup-computed-width: 40px;
margin-left: var(--p-space-4);
}

@include print-hidden;
@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}
}

.ActionMenuWrapper {
Expand All @@ -137,7 +152,10 @@ $action-menu-rollup-computed-width: 40px;
top: control-height() * 0.5;
}

@include print-hidden;
@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}
}

.Row {
Expand Down Expand Up @@ -193,7 +211,7 @@ $action-menu-rollup-computed-width: 40px;
margin-left: 0;
}

@include when-printing {
@media print {
// stylelint-disable-next-line declaration-no-important
margin-left: 0 !important;
}
Expand Down
5 changes: 4 additions & 1 deletion src/components/Popover/Popover.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,5 +119,8 @@ $vertical-motion-offset: -5px;
}

.PopoverOverlay-hideOnPrint {
@include hidden-when-printing;
@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}
}
1 change: 0 additions & 1 deletion src/styles/_common.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,3 @@
@import './shared/typography';
@import './shared/skeleton';
@import './shared/interaction-state';
@import './shared/printing';
1 change: 0 additions & 1 deletion src/styles/_public-api.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,3 @@
@import './shared/typography';
@import './shared/skeleton';
@import './shared/interaction-state';
@import './shared/printing';
19 changes: 0 additions & 19 deletions src/styles/shared/_printing.scss
View file
Open in desktop

This file was deleted.

7 changes: 0 additions & 7 deletions src/styles/shared/_typography.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,10 +165,3 @@ $typography-condensed: 640px;
white-space: nowrap;
text-overflow: ellipsis;
}

@mixin print-hidden {
@media print {
// stylelint-disable-next-line declaration-no-important
display: none !important;
}
}