Skip to content

Conversation

@dleroux
Copy link
Contributor

@dleroux dleroux commented Sep 13, 2019

WHY are these changes introduced?

We are bringing back the ability to have a non-primary/indigo button as the pages primary actions. We previously made the decision to remove this ability but there are use cases that having a non-indigo button does make sense.

WHAT is this pull request doing?

Making the default button indigo/primary but allowing the ability to make it a regular button.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Paste the code below in the playground and toggle primary key of the primaryAction.

Copy-paste this code in playground/Playground.tsx:
import React from 'react';
import {Page} from '../src';

export function Playground() {
  return (
    <Page
      breadcrumbs={[{content: 'Products'}]}
      title="Product reviews"
      primaryAction={{
        content: 'Save',
        disabled: true,
        primary: false,
      }}
      secondaryActions={[{content: 'Duplicate'}, {content: 'Upgrade'}]}
      actionGroups={[
        {
          title: 'Promote',
          actions: [
            {
              content: 'Share on Facebook',
            },
            {
              content: 'Share on Pinterest',
            },
          ],
        },
      ]}
    >
      <p>Page content</p>
    </Page>
  );
}

🎩 checklist

@dleroux dleroux force-pushed the allow-for-default-action branch from c036d88 to 51c3e92 Compare September 13, 2019 12:50
@dleroux dleroux force-pushed the allow-for-default-action branch from 51c3e92 to 7ff0fe4 Compare September 13, 2019 13:00
@dleroux dleroux changed the title [WIP][Page] Allow for non-primary page action [Page] Allow for non-primary page action Sep 13, 2019
@AndrewMusgrave
Copy link
Member

Would we want a primary action to be non-primary?

@dleroux dleroux requested a review from chloerice September 13, 2019 14:24
@sarahill
Copy link
Contributor

sarahill commented Sep 13, 2019

For additional context on this:

This was brought from @simon-callsen in the #polaris slack channel. A change had been made the page component that only allowed primary-buttons in the page header. This generally makes sense for the way the component was originally designed, but teams now have a need to use this in different ways that break that pattern.

The team discussed this in Slack and came to the conclusion that it's worth reverting that change and allowing non-primary actions in the page header.

There's a big opportunity here to revisit the component and consider how we can make this flexible for teams and their different use cases.

PR that introduced the change to allow only primary actions #1653

@sarahill sarahill self-requested a review September 13, 2019 14:33
Copy link
Contributor

@sarahill sarahill left a comment

Choose a reason for hiding this comment

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

This lgtm! Thanks for doing this, Dan ✨

Co-Authored-By: Chloe Rice <chloerice@users.noreply.github.com>
@dleroux dleroux merged commit ea2aa4b into master Sep 13, 2019
@dleroux dleroux deleted the allow-for-default-action branch September 13, 2019 15:25
@dleroux dleroux temporarily deployed to production September 20, 2019 17:44 Inactive
@dleroux dleroux temporarily deployed to production September 23, 2019 15:28 Inactive
@dleroux dleroux temporarily deployed to production September 23, 2019 15:38 Inactive
@dleroux dleroux temporarily deployed to production September 23, 2019 16:59 Inactive
@dleroux dleroux temporarily deployed to production September 23, 2019 17:05 Inactive
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.

4 participants