-
Notifications
You must be signed in to change notification settings - Fork 1.2k
[v4][Page] Narrow width page #1606
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,7 +25,7 @@ body { | |
max-width: none; | ||
} | ||
|
||
.singleColumn { | ||
.narrowWidth { | ||
max-width: layout-width(primary, max); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ export interface Props extends HeaderProps { | |
/** Remove the normal max-width on the page */ | ||
fullWidth?: boolean; | ||
/** Decreases the maximum layout width. Intended for single-column layouts */ | ||
singleColumn?: boolean; | ||
narrowWidth?: boolean; | ||
/** | ||
* Force render in page and do not delegate to the app bridge TitleBar action | ||
* @default false | ||
|
@@ -32,7 +32,14 @@ export interface Props extends HeaderProps { | |
forceRender?: boolean; | ||
} | ||
|
||
export type ComposedProps = Props & WithAppProviderProps; | ||
export interface DeprecatedProps { | ||
/** Decreases the maximum layout width. Intended for single-column layouts | ||
* @deprecated As of release 4.0, replaced by {@link https://polaris.shopify.com/components/structure/page#props-narrow-width} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't currently use these tags in the style guide, but ids should be added to the props none the less There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer this as well—using the correct tags and following standards. That way it is there if we want to support it in the future and/or change to tool we used to parse this. |
||
*/ | ||
singleColumn?: boolean; | ||
} | ||
|
||
export type ComposedProps = Props & DeprecatedProps & WithAppProviderProps; | ||
|
||
const APP_BRIDGE_PROPS: (keyof Props)[] = [ | ||
'title', | ||
|
@@ -90,12 +97,25 @@ export class Page extends React.PureComponent<ComposedProps, never> { | |
} | ||
|
||
render() { | ||
const {children, fullWidth, singleColumn, ...rest} = this.props; | ||
const { | ||
children, | ||
fullWidth, | ||
narrowWidth, | ||
singleColumn, | ||
...rest | ||
} = this.props; | ||
|
||
if (singleColumn) { | ||
// eslint-disable-next-line no-console | ||
console.warn( | ||
'Deprecation: The singleColumn prop has been renamed to narrowWidth to better represents its use and will be removed in v5.0.', | ||
); | ||
} | ||
|
||
const className = classNames( | ||
styles.Page, | ||
fullWidth && styles.fullWidth, | ||
singleColumn && styles.singleColumn, | ||
(narrowWidth || singleColumn) && styles.narrowWidth, | ||
); | ||
|
||
const headerMarkup = | ||
|
@@ -180,4 +200,4 @@ export class Page extends React.PureComponent<ComposedProps, never> { | |
} | ||
} | ||
|
||
export default withAppProvider<Props>()(Page); | ||
export default withAppProvider<Props & DeprecatedProps>()(Page); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -383,6 +383,20 @@ describe('<Page />', () => { | |
restoreTitleBarCreateMock(); | ||
}); | ||
}); | ||
|
||
describe('deprecations', () => { | ||
it('warns the singleColumn prop has been renamed', () => { | ||
const warningSpy = jest | ||
.spyOn(console, 'warn') | ||
.mockImplementation(() => {}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Blocking the warning spam in our test output |
||
mountWithAppProvider(<Page title="title" singleColumn />); | ||
|
||
expect(warningSpy).toHaveBeenCalledWith( | ||
'Deprecation: The singleColumn prop has been renamed to narrowWidth to better represents its use and will be removed in v5.0.', | ||
); | ||
warningSpy.mockRestore(); | ||
}); | ||
}); | ||
}); | ||
|
||
function noop() {} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wanted to, we could parse for this interface in the style guide (see bottom of the screenshot)