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

[Page] print mode adjustments #2469

Merged
merged 1 commit into from
Nov 26, 2019
Merged

[Page] print mode adjustments #2469

merged 1 commit into from
Nov 26, 2019

Conversation

patsissons
Copy link
Contributor

@patsissons patsissons commented Nov 25, 2019

WHY are these changes introduced?

When printing a page, we don't want the breadcrumbs or actions to be visible.

image

WHAT is this pull request doing?

removing actions and breadcrumbs from Page header in print mode using the print media query mixin.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Run the playground below and click the Print action to view the print mode page without any nav or actions.

Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Page} from '../src';
import {PrintMinor} from '@shopify/polaris-icons';

export function Playground() {
  return (
    <Page
      breadcrumbs={[{content: 'Breadcrumbs', url: '#'}]}
      title="Print demo"
      primaryAction={{content: 'Primary Action', url: '#'}}
      secondaryActions={[
        {content: 'Print', icon: PrintMinor, onAction: window.print},
        {content: 'Secondary Action', url: '#'},
      ]}
      actionGroups={[
        {title: 'Action groups', actions: [{content: 'Action', url: '#'}]},
      ]}
      pagination={{hasNext: true}}
    >
      <p>Page content</p>
    </Page>
  );
}

🎩 checklist

@github-actions
Copy link
Contributor

github-actions bot commented Nov 25, 2019

💦 Potential splash zone of changes introduced to src/**/*.tsx in this pull request:

Files modified2
Files potentially affected2

Details

All files potentially affected (total: 2)
📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🎨 src/components/Page/components/Header/Header.scss (total: 2)

Files potentially affected (total: 2)


This comment automatically updates as changes are made to this pull request.
Feedback, troubleshooting: open an issue or reach out on Slack in #polaris-tooling.

@patsissons patsissons force-pushed the page-print-media-query branch from eef862a to c8661d8 Compare November 26, 2019 17:23
@@ -9,6 +9,7 @@
- Added `ariaHaspopup` prop to `Popover` ([#2248](https://github.com/Shopify/polaris-react/pull/2248))
- Fixed an accessibility issue where the `Form` implicit submit was still accessible via keyboard ([#2447](https://github.com/Shopify/polaris-react/pull/2447))
- Moved `Button` styles from the `Buttongroup` CSS file to the `Button` CSS file ([#2441](https://github.com/Shopify/polaris-react/pull/2441))
- `Page` no longer renders navigation or actions in print mode ([#2469](https://github.com/Shopify/polaris-react/pull/2469))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should we consider this a breaking change?

Copy link
Member

Choose a reason for hiding this comment

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

I'd consider it a bug fix, as I think that you're right about this being expected behavior. @kaelig do you have an opinion on this?

@patsissons patsissons marked this pull request as ready for review November 26, 2019 17:24
@patsissons patsissons requested a review from chloerice November 26, 2019 17:24
Copy link
Member

@chloerice chloerice left a comment

Choose a reason for hiding this comment

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

🎩 Looks good 👍 🚀

@patsissons patsissons force-pushed the page-print-media-query branch from c8661d8 to 4e55853 Compare November 26, 2019 23:25
@patsissons patsissons merged commit e6c03de into master Nov 26, 2019
@patsissons patsissons deleted the page-print-media-query branch November 26, 2019 23:42
@dleroux dleroux temporarily deployed to production December 4, 2019 14:42 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants