Skip to content

Conversation

cboltt
Copy link
Contributor

@cboltt cboltt commented Nov 10, 2018

WHY are these changes introduced?

If a TopBar implements a userMenu but not a searchField, the userMenu is left-aligned. It should always remain right-aligned.

Resolves #566

WHAT is this pull request doing?

Sets justify-content: flex-end; on the TopBar's .Contents container (it's already using flexbox)

Before:
screen shot 2018-11-10 at 1 33 30 pm

After:
screen shot 2018-11-10 at 1 33 38 pm

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

The Top Bar component can take in three components as props: searchField, secondaryMenu, and userMenu. Be sure to test cases with different combinations of these props (i.e.: using a searchField and nothing else or a secondaryMenu and a userMenu but not a searchField).

Copy-paste this code in playground/Playground.tsx:
import * as React from 'react';
import {AppProvider, Frame, TopBar} from '@shopify/polaris';
import {noop} from '@shopify/javascript-utilities/other';

export default class Playground extends React.Component<never, never> {
  render() {
    const userMenuMarkup = (
      <TopBar.UserMenu
        actions={[]}
        name="Playground"
        detail="Wait, where's the slide?"
        initials="P"
        open={false}
        onToggle={noop}
      />
    );

    const secondaryMenuMarkup = (
      <TopBar.Menu
        activatorContent="Toggle menu"
        actions={[
          {
            items: [{content: 'item content'}],
          },
        ]}
        open={false}
        onClose={noop}
        onOpen={noop}
      />
    );

    const searchFieldMarkup = (
      <TopBar.SearchField onChange={noop} value="Search" placeholder="Search" />
    );

    const topBarMarkup = (
      <TopBar
        secondaryMenu={secondaryMenuMarkup}
        searchField={searchFieldMarkup}
        userMenu={userMenuMarkup}
      />
    );

    return (
      <AppProvider>
        <Frame topBar={topBarMarkup} />
      </AppProvider>
    );
  }
}

🎩 checklist

@ghost
Copy link

ghost commented Nov 10, 2018

👋 Thanks for opening your first pull request. A contributor should give feedback soon. If you haven’t already, please check out the contributing guidelines. You can also join #polaris on the Shopify Partners Slack.

@cboltt cboltt force-pushed the topbar-content-justify-flex-end branch from 151304f to 81e87d8 Compare November 10, 2018 18:46
@BPScott BPScott temporarily deployed to polaris-react-pr-597 November 10, 2018 18:46 Inactive
@cboltt cboltt self-assigned this Nov 12, 2018
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.

Code and tophat are 🥇
Rebase && 🚢 🚢 🚢

Copy link
Member

@BPScott BPScott left a comment

Choose a reason for hiding this comment

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

A tiny nitpick in the changelog message, but aside from that I love how easy this was to fix. Thanks!

Co-Authored-By: cbolton97 <chris.bolton97@gmail.com>
@BPScott BPScott requested a deployment to polaris-react-pr-597 November 13, 2018 22:22 Abandoned
@BPScott BPScott requested a deployment to polaris-react-pr-597 November 13, 2018 22:23 Abandoned
@cboltt cboltt merged commit e2d8cb3 into master Nov 13, 2018
@ghost
Copy link

ghost commented Nov 13, 2018

🎉 Thanks for your contribution to Polaris React!

@cboltt cboltt deleted the topbar-content-justify-flex-end branch November 13, 2018 22:27
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