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

feat: button link ts conversion #1280

Merged
merged 12 commits into from Oct 5, 2020
Merged

Conversation

EdwardIrby
Copy link
Contributor

ts conversion link and button

@EdwardIrby EdwardIrby marked this pull request as ready for review October 5, 2020 01:34
@EdwardIrby EdwardIrby changed the title --wip-- [skip ci] feat: button link ts conversion Oct 5, 2020
EdwardIrby and others added 3 commits October 5, 2020 08:10
delete filteredProps.onClick
}

return React.createElement(
Copy link
Contributor

Choose a reason for hiding this comment

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

i wonder why we were using createElement....guessing this was one of the first components and maybe we didn't support jsx then. just thinking out load - don't change anything for this pr but i might follow up with a conversion to jsx...

Comment on lines 211 to 221
Button.propTypes = {
appearance: PropTypes.oneOf(Object.keys(vars.appearances)),
children: PropTypes.any,
disabled: PropTypes.bool,
href: PropTypes.string,
icon: PropTypes.element,
iconAlign: PropTypes.oneOf(Object.keys(vars.iconAligns)),
loading: PropTypes.bool,
size: PropTypes.oneOf(Object.keys(vars.sizes)),
style: PropTypes.object
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Button.propTypes = {
appearance: PropTypes.oneOf(Object.keys(vars.appearances)),
children: PropTypes.any,
disabled: PropTypes.bool,
href: PropTypes.string,
icon: PropTypes.element,
iconAlign: PropTypes.oneOf(Object.keys(vars.iconAligns)),
loading: PropTypes.bool,
size: PropTypes.oneOf(Object.keys(vars.sizes)),
style: PropTypes.object
}
Button.propTypes = {
appearance: PropTypes.oneOf(Object.keys(vars.appearances)),
children: PropTypes.any,
disabled: PropTypes.bool,
href: PropTypes.string,
icon: PropTypes.element,
iconAlign: PropTypes.oneOf(Object.keys(vars.iconAligns)),
loading: PropTypes.bool,
size: PropTypes.oneOf(Object.keys(vars.sizes)),
style: PropTypes.object
}

Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think we need to add proptypes anymore. if we want to support this in the future we should generate them from the ts typings

@EdwardIrby EdwardIrby merged commit d15d64d into master Oct 5, 2020
@EdwardIrby EdwardIrby deleted the feat/link-button-ts-conversion branch October 5, 2020 20:33
danethurber added a commit that referenced this pull request Oct 7, 2020
…to ts/verttabs

* 'ts/verttabs' of github.com:pluralsight/design-system: (33 commits)
  build: fix secvulns by forcing package resolution
  refactor(docs): use postcss-env to declare media vars
  refactor(datepicker): update related snapshots
  feat(docs): linked headings in mdx
  fix(docs): single line commits to fix transform in preview
  fix(docs): properly label dev workflow lang types
  fix(docs): rm shell-session from supported renderable langs
  fix(docs): rm slack in the props, lol
  refactor(docs): give side nav custom scroller
  feat(docs): add css ssr plugins
  feat(docs): supply deps to new codesandboxes
  refactor(datepicker): adjust min-width
  feat: button link ts conversion (#1280)
  refactor(badge): use keymirror in vars
  test(badge): rm js snapshot
  build(badge): correct build scripts for ts
  refactor(badge): remove index.js refs
  refactor(badge): rm filterReactProps
  build(badge): cp verticaltabs latest tsconfig.json
  build(badge): mv index.js to index.ts
  ...
@jaketrent jaketrent mentioned this pull request Oct 13, 2020
53 tasks
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.

None yet

2 participants