Skip to content

[Page] Sync small screen header with native mobile #707

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 1 commit into from
Dec 20, 2018
Merged

[Page] Sync small screen header with native mobile #707

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Use [the changelog guidelines](https://git.io/polaris-changelog-guidelines) to f
- Moved icons to a separate npm package ([#686](https://github.com/Shopify/polaris-react/pull/686))
- Added `oneHalf` and `oneThird` props to `Layout` component ([#724](https://github.com/Shopify/polaris-react/pull/724))
- Added `helpText` prop to ActionList items ([#777](https://github.com/Shopify/polaris-react/pull/777))
- Updated `Page` header layout so actions take up less room on small screens ([#707](https://github.com/Shopify/polaris-react/pull/707))

### Bug fixes

Expand Down
37 changes: 34 additions & 3 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 @@ -5,8 +5,9 @@ $action-horizontal-padding: (1.5 * spacing(tight));
}

.Header-hasPagination,
.Header-hasBreadcrumbs {
@include page-header-has-pagination-and-breadcrumbs;
.Header-hasBreadcrumbs,
.Header-hasRollup {
@include page-header-has-navigation;
}

.Header-hasSecondaryActions {
Expand All @@ -23,18 +24,34 @@ $action-horizontal-padding: (1.5 * spacing(tight));
}

.Header-hasRollup {
.SecondaryActions {
display: none;
}

.IndividualActions {
display: none;
}

.Pagination {
display: none;
}

@include page-content-when-not-partially-condensed {
.Rollup {
display: none;
}

.SecondaryActions {
display: flex;
}

.IndividualActions {
display: flex;
}

.Pagination {
display: flex;
}
}
}

Expand All @@ -44,7 +61,6 @@ $action-horizontal-padding: (1.5 * spacing(tight));

.PrimaryAction {
display: flex;
align-self: stretch;
align-items: center;

.MainContent > & {
Expand All @@ -61,6 +77,10 @@ $action-horizontal-padding: (1.5 * spacing(tight));
}
}

.Rollup {
margin-top: spacing(extra-tight);
}

.Pagination {
margin-left: auto;
line-height: 1;
Expand All @@ -73,6 +93,7 @@ $action-horizontal-padding: (1.5 * spacing(tight));
.Navigation {
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: spacing(extra-tight);
}

Expand All @@ -81,6 +102,16 @@ $action-horizontal-padding: (1.5 * spacing(tight));
align-items: center;
}

.TitleAndRollup {
display: flex;
justify-content: space-between;
align-items: baseline;

.Rollup {
@include page-header-without-navigation;
}
}

.TitleAndActions {
flex: 1 1 0%;
}
Expand Down
76 changes: 45 additions & 31 deletions src/components/Page/components/Header/Header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,13 +122,14 @@ class Header extends React.PureComponent<CombinedProps, State> {
</div>
) : null;

const rollupMarkup = this.renderRollupAction();
const nonPrimaryActionsMarkup = this.renderSecondaryActions();

const actionsMarkup =
primaryAction || secondaryActions || actionGroups ? (
<div className={styles.Actions}>
{primaryActionMarkup}
{nonPrimaryActionsMarkup}
{primaryActionMarkup}
</div>
) : null;

Expand All @@ -137,18 +138,22 @@ class Header extends React.PureComponent<CombinedProps, State> {
<div className={styles.Navigation}>
{breadcrumbMarkup}
{paginationMarkup}
{breadcrumbMarkup && rollupMarkup}
</div>
) : null;

const titleMarkup = (
<div className={styles.Title}>
{/* Anonymous divs are here for layout purposes */}
<div>
<DisplayText size="large" element="h1">
{title}
</DisplayText>
<div className={styles.TitleAndRollup}>
<div className={styles.Title}>
{/* Anonymous divs are here for layout purposes */}
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a side note, no need to do anything here, but when I see a comment like this I wonder if we can instead write the code in a more meaningful way like creating a component called AnonymousDivForLayout that just renders a div.

<div>
<DisplayText size="large" element="h1">
{title}
</DisplayText>
</div>
<div>{titleMetadata}</div>
</div>
<div>{titleMetadata}</div>
{!breadcrumbMarkup && rollupMarkup}
</div>
);

Expand All @@ -174,11 +179,41 @@ class Header extends React.PureComponent<CombinedProps, State> {

private get hasRollup() {
const {secondaryActions = [], actionGroups = []} = this.props;
return secondaryActions.length + actionGroups.length > 1;
return secondaryActions.length + actionGroups.length >= 1;
}

@autobind
private renderRollupAction() {
const {rollupOpen} = this.state;
const {secondaryActions = [], actionGroups = []} = this.props;
const rollupMarkup = this.hasRollup ? (
<div className={styles.Rollup}>
<Popover
active={rollupOpen}
onClose={this.handleRollupToggle}
activator={
<Button
plain
icon="horizontalDots"
onClick={this.handleRollupToggle}
/>
}
>
<ActionList
items={secondaryActions}
sections={actionGroups.map(convertActionGroupToActionListSection)}
onActionAnyItem={this.handleRollupToggle}
/>
</Popover>
</div>
) : null;

return rollupMarkup;
}

@autobind
private renderSecondaryActions() {
const {openActionGroup, rollupOpen} = this.state;
const {openActionGroup} = this.state;
const {secondaryActions = [], actionGroups = []} = this.props;

if (secondaryActions.length === 0 && actionGroups.length === 0) {
Expand Down Expand Up @@ -210,29 +245,8 @@ class Header extends React.PureComponent<CombinedProps, State> {
))
: null;

const rollupMarkup = this.hasRollup ? (
<div className={styles.Rollup}>
<Popover
active={rollupOpen}
onClose={this.handleRollupToggle}
activator={
<Button disclosure onClick={this.handleRollupToggle}>
Actions
</Button>
}
>
<ActionList
items={secondaryActions}
sections={actionGroups.map(convertActionGroupToActionListSection)}
onActionAnyItem={this.handleRollupToggle}
/>
</Popover>
</div>
) : null;

return (
<div className={styles.SecondaryActions}>
{rollupMarkup}
<div className={styles.IndividualActions}>
{secondaryActionMarkup}
{actionGroupsMarkup}
Expand Down
9 changes: 6 additions & 3 deletions src/styles/shared/page.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,21 @@ $actions-vertical-spacing: spacing(tight);
}
}

@mixin page-header-has-pagination-and-breadcrumbs {
@mixin page-header-has-navigation {
padding-top: rem(32px);
}

@mixin page-header-without-navigation {
margin-top: unset;
}

@mixin page-header-has-secondary-actions {
padding-top: rem(24px);
}

@mixin page-actions-layout {
display: flex;
flex-direction: row-reverse;
justify-content: flex-end;
justify-content: flex-start;
align-items: center;
margin-top: 2 * $actions-vertical-spacing;

Expand Down