components icon indicating copy to clipboard operation
components copied to clipboard

[wizardlayout] support any button type

Open prime-time opened this issue 6 years ago • 2 comments

Overview: This change lets us use any button type for the WizardLayout's next button. I saw a couple of cases in some of our design mocks where the secondary button type is used. I'm not sure if we'd ever actually use the other button types, but they'll be there if we want them.

Screenshots/GIFs: Here's an example of a design mock that uses a secondary button type for the next button.

Testing:

  • [ ] Unit tests
  • Manual tests:
    • [ ] Chrome
    • [ ] Safari
    • [ ] IE10

Roll Out:

  • Before merging:
    • [ ] Updated docs
    • [ ] Bumped version in package.json
      • Breaking change? Run npm version major
      • New component or backward-compatible component feature change? Run npm version minor
      • Only changing documentation? All good. Skip this step.
    • After creating a new component, make sure to add it to the Components List in ComponentsView.jsx. To do so:
      • [ ] Add an entry in ComponentsView.componentsToDisplay using this template:
        {
          componentLink: "<COMPONENT LINK>",
          componentImg: "<COMPONENT LINK>.png",
          componentName: "<COMPONENT NAME>",
          componentImgAlt: "A <COMPONENT NAME> component",
        },
        
      • [ ] Add a screenshot of the component in docs/assets/img with the format <COMPONENT LINK>.png
  • After merging:
    • [ ] Deployed updated docs (make deploy-docs)

prime-time avatar Feb 20 '20 22:02 prime-time

Instead of adding another prop here, I would consider inversion of control https://kentcdodds.com/blog/inversion-of-control

josh-clever avatar Feb 21 '20 00:02 josh-clever

Instead of adding another prop here, I would consider inversion of control https://kentcdodds.com/blog/inversion-of-control

Thanks for sharing that article. I agree that adding an extra prop isn't the ideal solution. If we applied the principles from the article, we could have say a primaryButton prop that takes in a button element: <Button type="primary" value="something" />, or maybe we could let the client pass in the entire footer.

I might not need this change, so I may close this pull request, but it's good to think about how we might make the API more flexible for the future.

prime-time avatar Feb 21 '20 11:02 prime-time