Skip to content
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

refactor: Update StaticStatesTable to be more flexible #418

Merged
merged 1 commit into from
Jan 23, 2020

Conversation

anicholls
Copy link
Contributor

@anicholls anicholls commented Jan 23, 2020

Summary

Based on the feedback in #372, it was decided to make the StaticStatesTable more flexible and generic. This includes the following changes:

  • Rename to ComponentStatesTable
  • The table no longer includes StaticStates by default, so you could use it for any components with variable props. You should now wrap the entire table in a StaticStates component
  • states is now columnProps and is treated the same way as rowProps
  • Permutation is now done by a utility function called permutateProps instead of within the table itself. This should be the standard way you would use the table. The shouldRenderRow function has changed to a filter function (the second argument of permutateProps)
    • This function returns a list of prop combinations, which you could pass in directly if you find any confusing filtering use cases.
  • ComponentStatesTable now uses the child as a render function rather than an explicit renderComponent prop.

This should go in ASAP as it's blocking #386, #381, #412, #394, #390, and #407

Checklist

  • branch has been rebased on the latest master commit
  • tests are changed or added
  • yarn test passes
  • code has been documented and, if applicable, usage described in README.md

Additional References

Example usage:

<StaticStates>
  <ComponentStatesTable
    rowProps={permutateProps( // permutateProps example
      {
        checked: [{value: true, label: 'Checked'}, {value: false, label: 'Unchecked'}],
        indeterminate: [{value: true, label: 'Indeterminate'}, {value: false, label: ''}],
        error: [
          {value: undefined, label: ''},
          {value: Checkbox.ErrorType.Alert, label: 'Alert'},
          {value: Checkbox.ErrorType.Error, label: 'Error'},
        ],
      },
      props => {
        if (props.indeterminate && !props.checked) {
          return false;
        }
        return true;
      }
    )}
    columnProps={[  // List of props example (matches the output of permutateProps)
      {label: 'Default', props: {className: ''}},
      {label: 'Disabled', props: {className: '', disabled: true}},
      {label: 'Hover', props: {className: 'hover'}},
      {label: 'Hover Disabled', props: {className: 'hover', disabled: true}},
      {label: 'Focus', props: {className: 'focus'}},
      {label: 'Focus Hover', props: {className: 'focus hover'}},
      {label: 'Active', props: {className: 'active'}},
      {label: 'Active Hover', props: {className: 'active hover'}},
    ]}
  >
    {props => (
      <Checkbox
        {...props}
        onChange={() => {}} // eslint-disable-line no-empty-function
        label="Checkbox"
      />
    )}
  </ComponentStatesTable>
</StaticStates>

As you can see, the list of props can be more simple for some cases. However, caution should be taken as it's easy to forget permutations. I would suggest only using it for pseudostates/classnames in the columnProps and not for component props.

@anicholls anicholls added enhancement New feature or request infra p:1 labels Jan 23, 2020
@cypress
Copy link

cypress bot commented Jan 23, 2020



Test summary

65 0 0 0


Run details

Project canvas-kit
Status Passed
Commit 675a1ed
Started Jan 23, 2020 1:17 AM
Ended Jan 23, 2020 1:19 AM
Duration 01:17 💡
OS Linux Ubuntu Linux - 16.04
Browser Electron 61

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@NicholasBoll NicholasBoll merged commit 8e3297b into Workday:master Jan 23, 2020
@lychyi lychyi added this to In progress (PRs) in Current Sprint (7/20 - 8/9) via automation Jan 23, 2020
@lychyi lychyi moved this from In progress (PRs) to Done in Current Sprint (7/20 - 8/9) Jan 23, 2020
@anicholls anicholls deleted the static-states-table branch January 24, 2020 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request infra
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants