Skip to content

Conversation

@alex-page
Copy link
Member

WHY are these changes introduced?

This is a part of https://github.com/Shopify/polaris-rails/issues/2182

WHAT is this pull request doing?

Changes the fontsize and breakpoints for the page title in the new design language.

🎩 checklist

  • Tested on mobile
  • Tested on multiple browsers
  • Tested for accessibility
  • Updated the component's README.md with documentation changes
  • Tophatted documentation changes in the style guide
  • For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

@alex-page alex-page self-assigned this Oct 13, 2020
@github-actions
Copy link
Contributor

🟢 This pull request modifies 2 files and might impact 3 other files.

Details:
All files potentially affected (total: 3)
🎨 src/components/Page/components/Header/components/Title/Title.scss (total: 3)

Files potentially affected (total: 3)

🧩 src/components/Page/components/Header/components/Title/Title.tsx (total: 2)

Files potentially affected (total: 2)

@alex-page alex-page added the 🤖Skip Changelog Causes CI to ignore changelog update check. label Oct 13, 2020
import type {ThumbnailProps} from '../../../../../Thumbnail';
import {classNames} from '../../../../../../utilities/css';
import {useFeatures} from '../../../../../../utilities/features';
import {useMediaQuery} from '../../../../../../utilities/media-query';
Copy link
Member Author

@alex-page alex-page Oct 13, 2020

Choose a reason for hiding this comment

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

This hook allows React to change when the mobile menu icon shows. In this case we do not want to change the font size on this breakpoint, we want to change it on the smallest breakpoint.

Copy link
Member

@kyledurand kyledurand 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 to me 👍

Copy link
Contributor

@francinen francinen left a comment

Choose a reason for hiding this comment

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

Is this the expected behaviour? If it is, looks good 👍

page-title-size

@alex-page
Copy link
Member Author

Yeah that is expected, before we had three title sizes and there should only be two. Thanks @francinen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖Skip Changelog Causes CI to ignore changelog update check.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants