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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[ThemeProvider] Change frameOffset to accept a string #3883

Merged
merged 1 commit into from Jan 19, 2021
Merged

[ThemeProvider] Change frameOffset to accept a string #3883

merged 1 commit into from Jan 19, 2021

Conversation

kyledurand
Copy link
Contributor

WHY are these changes introduced?

Fixes https://github.com/Shopify/global-nav/issues/843

WHAT is this pull request doing?

Some consumers need relative units for the frame offset. This takes into account for font-size and matching up breakpoints.

This PR allows for any string to be passed in i.e. 12em and still defaults to 0px.

How to 馃帺

Check out the frame example and try different units

馃帺 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 @鈥奌YPD, @鈥妋irualves, @鈥妔arahill, or @鈥妑y5n to update the Polaris UI kit

@kyledurand kyledurand self-assigned this Jan 19, 2021
@kyledurand kyledurand force-pushed the frame-offset_accept-string-value-instead-of-number branch from 638d66c to 8b4b5c7 Compare January 19, 2021 20:43
@kyledurand kyledurand force-pushed the frame-offset_accept-string-value-instead-of-number branch from 8b4b5c7 to 4deb074 Compare January 19, 2021 20:44
@github-actions
Copy link
Contributor

github-actions bot commented Jan 19, 2021 •
edited

馃煛 This pull request modifies 5 files and might impact 68 other files. This is an average splash zone for a change, remember to tophat areas that could be affected.

Details:
All files potentially affected (total: 68)
馃搫 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

馃搫 src/components/Frame/README.md (total: 0)

Files potentially affected (total: 0)

馃З src/utilities/theme/tests/utils.test.ts (total: 0)

Files potentially affected (total: 0)

馃З src/utilities/theme/types.ts (total: 68)

Files potentially affected (total: 68)

馃З src/utilities/theme/utils.ts (total: 65)

Files potentially affected (total: 65)

CSilivestru
CSilivestru approved these changes Jan 19, 2021
View reviewed changes
Copy link
Contributor

@CSilivestru CSilivestru left a comment

Choose a reason for hiding this comment

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

馃敟
Thanks so much for such a quick turnaround on this!

@kyledurand kyledurand merged commit 8a3d674 into version-6.0.0 Jan 19, 2021
@kyledurand kyledurand deleted the frame-offset_accept-string-value-instead-of-number branch January 19, 2021 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CSilivestru CSilivestru CSilivestru approved these changes

@alex-page alex-page alex-page approved these changes

Assignees

@kyledurand kyledurand

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@kyledurand @CSilivestru @alex-page