style(Table|mixed): update typings, tests and propTypes usage #1200

layershifter · 2017-01-21T14:39:44Z

This PR promised to be small, but...

It contains:

typings update for Table for fix(typings): unable to set many typical html element props #1072 & Add docs for sortable Table and sorted TableCell #1195;
updates propTypes usage for RFC: remove production propTypes, allow small custom builds #524;
update typings type SemanticWIDTHS;
adds jsdoc to all Table's subcomponents;
updates propKeyAndValueToClassName test.

layershifter · 2017-01-21T14:40:23Z

src/collections/Form/index.d.ts

@@ -54,7 +53,7 @@ interface FormProps {
  warning?: boolean;

  /** Forms can automatically divide fields to be equal width */
-  widths?: SemanticWIDTHSSTRING | SemanticWIDTHSNUMBER | number | 'equal';
+  widths?: SemanticWIDTHS | 'equal';


I've created new type SemanticWIDTHS, it's more strict and compact.

layershifter · 2017-01-21T14:40:57Z

src/collections/Table/Table.js

-    compact: ['very'],
-    padded: ['very'],
-    size: _.without(SUI.SIZES, 'mini', 'tiny', 'medium', 'big', 'huge', 'massive'),
-  },


_meta.props removal 😄

layershifter · 2017-01-21T14:41:17Z

src/collections/Table/TableCell.js

@@ -16,6 +16,9 @@ import {
 } from '../../lib'
 import Icon from '../../elements/Icon'

+/**
+ * A table row can have cells.
+ */


Added jsdoc for subcomponents.

layershifter · 2017-01-21T14:41:45Z

src/collections/Table/TableHeaderCell.js

 function TableHeaderCell(props) {
  const { as, className, sorted } = props
  const classes = cx(
    useValueAndKey(sorted, 'sorted'),
    className
  )
  const rest = getUnhandledProps(TableHeaderCell, props)
-  return <TableCell as={as} {...rest} className={classes} />
+
+  return <TableCell {...rest} as={as} className={classes} />


{...rest} must be first.

Usually, when spreading this is true. But, since rest excludes as and className due to getUnhandledProps there is no danger of overwriting. I still think it is good to move it to the first position though, 👍

layershifter · 2017-01-21T14:45:28Z

test/specs/commonTests.js

@@ -873,7 +873,7 @@ export const implementsTextAlignProp = (Component, alignments = SUI.TEXT_ALIGNME
    _noDefaultClassNameFromProp(Component, 'textAlign', options)
    _noClassNameFromBoolProps(Component, 'textAlign', options)

-    _.each(alignments, propVal => {
+    alignments.forEach(propVal => {


Lodash's each will work even alignments will not be array, we need to be more strict there.

layershifter · 2017-01-21T14:45:47Z

test/specs/commonTests.js

 * @param {Object} [options={}]
 * @param {Object} [options.requiredProps={}] Props required to render the component.
 * @param {Object} [options.className=propKey] The className to assert exists.
 */
-export const propValueOnlyToClassName = (Component, propKey, options = {}) => {
+export const propValueOnlyToClassName = (Component, propKey, propValues, options = {}) => {


Updated test.

layershifter · 2017-01-21T14:46:08Z

test/specs/elements/Image/Image-test.js

@@ -19,7 +20,7 @@ describe('Image Component', () => {
    ShorthandComponent: Dimmer,
    mapValueToProps: val => ({ content: val }),
  })
-  common.implementsVerticalAlignProp(Image, 'verticalAlign')


layershifter · 2017-01-21T14:59:28Z

src/collections/Table/TableCell.js

@@ -122,16 +120,16 @@ TableCell.propTypes = {
  singleLine: PropTypes.bool,

  /** A table cell can adjust its text alignment. */
-  textAlign: PropTypes.oneOf(TableCell._meta.props.textAlign),
+  textAlign: PropTypes.oneOf(_.without(SUI.TEXT_ALIGNMENTS, 'justified')),


Table doesn't have justify.

layershifter · 2017-01-21T14:59:58Z

src/elements/Segment/Segment.js

@@ -148,7 +148,7 @@ Segment.propTypes = {
  tertiary: PropTypes.bool,

  /** Formats content to be aligned as part of a vertical group. */
-  textAlign: PropTypes.oneOf(SUI.TEXT_ALIGNMENTS),
+  textAlign: PropTypes.oneOf(_.without(SUI.TEXT_ALIGNMENTS, 'justified')),


Segment doesn't have justify.

codecov-io · 2017-01-22T22:07:24Z

Current coverage is 95.89% (diff: 100%)

No coverage report found for master at af12695.

Powered by Codecov. Last update af12695...515878b

levithomason

Awesome, I'm so grateful for all the work you're putting into this. It is immensely helpful and much appreciated. If you weren't doing this, it wouldn't be getting done. 🎉

levithomason · 2017-01-24T16:38:40Z

src/collections/Table/TableHeaderCell.js

 function TableHeaderCell(props) {
  const { as, className, sorted } = props
  const classes = cx(
    useValueAndKey(sorted, 'sorted'),
    className
  )
  const rest = getUnhandledProps(TableHeaderCell, props)
-  return <TableCell as={as} {...rest} className={classes} />
+
+  return <TableCell {...rest} as={as} className={classes} />


Usually, when spreading this is true. But, since rest excludes as and className due to getUnhandledProps there is no danger of overwriting. I still think it is good to move it to the first position though, 👍

* Semantic-Org/master: (111 commits) fix(docs): fix usage of arrow function (Semantic-Org#1228) fix(Icon): Incorrect definition in typings (Semantic-Org#1225) fix(Button): fix `tabIndex` in typings (Semantic-Org#1224) fix(typings): add item prop to the DropdownProps (Semantic-Org#1223) docs(examples): add missing `key` props (Semantic-Org#1220) docs(changelog): update changelog [ci skip] 0.64.4 feat(Form): add `inverted` prop (Semantic-Org#1218) fix(ComponentExample): use explicit babel presets (Semantic-Org#1219) style(Button): update typings and propTypes usage (Semantic-Org#1216) style(Embed): update typings and propTypes usage (Semantic-Org#1217) chore(package): support for jsnext:main (Semantic-Org#1210) style(Step): update typings, tests and propTypes usage (Semantic-Org#1203) fix(Portal): do not take focus after first render (Semantic-Org#1215) style(Table|mixed): update typings, tests and propTypes usage (Semantic-Org#1200) fix(Dropdown): use `item` instead of `as={Menu.Item}` (Semantic-Org#659) fix(Dropdown): respect `closeOnBlur={false}` (Semantic-Org#1148) style(Breadcrumb): update typings and propTypes usage (Semantic-Org#1209) style(Dimmer): update typings (Semantic-Org#1208) fix(Divider|FormInput): fix the broken typings (Semantic-Org#1179) ...

…ic-Org#1200) * style(Table|mixed): update typings, tests and propTypes usage * style(Table): add missing prop to typings

style(Table|mixed): update typings, tests and propTypes usage

4c0e57a

layershifter added the ready for review label Jan 21, 2017

layershifter commented Jan 21, 2017

View reviewed changes

This was referenced Jan 21, 2017

fix(typings): unable to set many typical html element props #1072

Closed

RFC: remove production propTypes, allow small custom builds #524

Closed

layershifter commented Jan 21, 2017

View reviewed changes

style(Table): add missing prop to typings

515878b

levithomason approved these changes Jan 24, 2017

View reviewed changes

levithomason merged commit efd079b into master Jan 24, 2017

levithomason deleted the style/table-typings branch January 24, 2017 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

style(Table|mixed): update typings, tests and propTypes usage #1200

style(Table|mixed): update typings, tests and propTypes usage #1200

layershifter commented Jan 21, 2017

layershifter Jan 21, 2017

layershifter Jan 21, 2017

layershifter Jan 21, 2017

layershifter Jan 21, 2017

levithomason Jan 24, 2017

layershifter Jan 21, 2017

layershifter Jan 21, 2017

layershifter Jan 21, 2017

layershifter Jan 21, 2017

layershifter Jan 21, 2017

codecov-io commented Jan 22, 2017

levithomason left a comment

levithomason Jan 24, 2017

style(Table|mixed): update typings, tests and propTypes usage #1200

style(Table|mixed): update typings, tests and propTypes usage #1200

Conversation

layershifter commented Jan 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Jan 22, 2017

Current coverage is 95.89% (diff: 100%)

levithomason left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment