Header Component updated to v1 API #278

jhchill666 · 2016-06-24T11:41:39Z

Updates the Header component to v1 Api
Examples updated to match Semantic Docs
Multiple tests for Header variants added to suite

- Updates the Header component to v1 Api - Examples updated to match Semantic Docs - Multiple tests for Header variants added to suite

codecov-io · 2016-06-24T11:46:50Z

Current coverage is 87.76%

Merging #278 into master will increase coverage by 2.06%

@@             master       #278   diff @@
==========================================
  Files            62         62          
  Lines           797        817    +20   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            683        717    +34   
+ Misses          114        100    -14   
  Partials          0          0

Powered by Codecov. Last updated by 312c234...20bf1ad

levithomason · 2016-06-24T16:34:39Z

src/elements/Header/HeaderSubheader.js

+  const rest = getUnhandledProps(HeaderSubheader, props)
+
+  return (
+    <SubHeaderComponent className={classes} {...rest}>


Since the component never changes, let's inline the div here:

<div ...

Done

Levi Thomason mailto:notifications@github.com
24 June 2016 at 17:34

In src/elements/Header/HeaderSubheader.js
https://github.com/TechnologyAdvice/stardust/pull/278#discussion_r68424700:

+function HeaderSubheader(props) {

const {

children, className,

} = props
+

const classes = cx('sd-header-subheader', 'sub',

className,

'header'

)
+

const SubHeaderComponent = 'div'

const rest = getUnhandledProps(HeaderSubheader, props)
+

return (

<SubHeaderComponent className={classes} {...rest}>

Since the component never changes, let's inline the div here:

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/TechnologyAdvice/stardust/pull/278/files/4317290b6fd2e4980cfd74e736287438f69d4c6c#r68424700,
or mute the thread
https://github.com/notifications/unsubscribe/AAWHs7_dvxGO4qHx5pDEs0ZBC30NYrf5ks5qPAcggaJpZM4I9saB.

Sent from Postbox
https://www.postbox-inc.com/?utm_source=email&utm_medium=siglink&utm_campaign=reach

levithomason · 2016-06-24T16:50:54Z

src/elements/Header/_Header.js

+    useKeyOnly(justified, 'justified'),
+    useKeyOnly(dividing, 'dividing'),
+    useKeyOnly(block, 'block'),
+    useKeyOnly(attached, 'attached'),


The attached option appears as top attached, attached, and bottom attached html classes. In this case the key or the value and key are used. They are express in the API as:

<Header attached /> <Header attached='top' /> <Header attached='bottom' />

Then in the className buildup as:

cx( useKeyOrValueAndKey(attached, 'attached'), )

This method will use the boolean value (attached) as the className if no value was provided. It will also handle the value if provided. Hence the awkward but accurate naming "use key or value and key".

For comparison, the current implementation would always produce this for all the above API permutations:

<div class="ui attached header"></div>

Because only the key "attached" is being used. Use useKeyOnly for boolean style classes, classes that can appear in tandem with other classes, are either present or not, and do not have any combination classes.

Label seems to use useValueAndKey(attached, 'attached'), rather than
what you specify below. Is there reason for this?

Levi Thomason mailto:notifications@github.com
24 June 2016 at 17:50

In src/elements/Header/_Header.js
https://github.com/TechnologyAdvice/stardust/pull/278#discussion_r68426699:

+function _Header(props) {

const {

_sdClass, _headerElement,

left, center, right, justified, dividing, block, attached, floating,

icon, image, children, className,

} = props
+

const classes = cx(

_sdClass, 'ui',

useKeyOnly(left, 'left aligned'),

useKeyOnly(center, 'center aligned'),

useKeyOnly(right, 'right aligned'),

useKeyOnly(justified, 'justified'),

useKeyOnly(dividing, 'dividing'),

useKeyOnly(block, 'block'),

useKeyOnly(attached, 'attached'),

The attached http://semantic-ui.com/elements/header#attached option
appears as |top attached|, |attached|, and |bottom attached| html
classes. In this case the key /or/ the value and key are used. They
are express in the API as:

Then in the className buildup as:

cx(
useKeyOrValueAndKey(attached, 'attached'),
)

This method will use the boolean value (attached) as the className if
no value was provided. It will also handle the value if provided.
Hence the awkward but accurate naming "use key /or/ value and key".

For comparison, the current implementation would always produce this
for all the above API permutations:

Because only the key "attached" is being used. Use |useKeyOnly| for
boolean style classes, classes that can appear in tandem with other
classes, are either present or not, and do not have any combination
classes.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/TechnologyAdvice/stardust/pull/278/files/4317290b6fd2e4980cfd74e736287438f69d4c6c#r68426699,
or mute the thread
https://github.com/notifications/unsubscribe/AAWHsxeoP7PeGInj9OAf7QvegYpGi44zks5qPArwgaJpZM4I9saB.

Sent from Postbox
https://www.postbox-inc.com/?utm_source=email&utm_medium=siglink&utm_campaign=reach

…ation for attached prop

levithomason · 2016-06-24T16:56:19Z

src/elements/Header/_Header.js

+    icon, image, children, className,
+  } = props
+
+  const classes = cx(


Let's add support for disabled and color:

<Header disabled /> cx( useKeyOnly(disabled, 'disabled') )

<Header color='red' /> cx( color, // since only the value is used, there is no helper method )

I think it's becoming apparent, my inferior knowledge of the Semantic
prop types needs a bit of work. It might aid the process, putting
together a doc of definitive prop types for all components. This might
also help to visualise how these can be simplified across all components
and might help make the api as user friendly as possible?

I'd be happy to undertake this, as would help my knowledge if helping
migrate these components?

Levi Thomason mailto:notifications@github.com
24 June 2016 at 17:56

In src/elements/Header/_Header.js
https://github.com/TechnologyAdvice/stardust/pull/278#discussion_r68427374:

import META from '../../utils/Meta'
+import {

getUnhandledProps,

iconPropRenderer,

imagePropRenderer,

useKeyOnly,
+} from '../../utils/propUtils'
+
+function _Header(props) {

const {

_sdClass, _headerElement,

left, center, right, justified, dividing, block, attached, floating,

icon, image, children, className,

} = props
+

const classes = cx(

Let's add support for |disabled|
http://semantic-ui.com/elements/header#disabled and |color|
http://semantic-ui.com/elements/header#colored:

cx(
useKeyOnly(disabled, 'disabled')
)

cx(
color, // since only the value is used, there is no helper method
)

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/TechnologyAdvice/stardust/pull/278/files/4317290b6fd2e4980cfd74e736287438f69d4c6c#r68427374,
or mute the thread
https://github.com/notifications/unsubscribe/AAWHsy7SjrIp2M26spj0QJyzBLkFsA6Mks5qPAw0gaJpZM4I9saB.

Sent from Postbox
https://www.postbox-inc.com/?utm_source=email&utm_medium=siglink&utm_campaign=reach

That is a great idea and it's been on my list. Thanks for trudging through this in it's absence.

I have started some notes in the actual code along side the prop utils themselves here:
https://github.com/TechnologyAdvice/stardust/blob/master/src/utils/propUtils.js#L76

In my head I see a simple markdown file that explains to folks "How to write a component". One of the first orders of business in that doc is the explanation of the component API and the 4 different prop to className permutations (noted in the code linked above).

I think even starting a markdown file with just that ^ information in it for now is a good start. We can iterate and add as we go. I'll keep this on my list, but if you get to it before me I'll gladly review and contribute.

Issue created: https://github.com/TechnologyAdvice/stardust/issues/282

tests updated to reflect examples updated to reflect

levithomason · 2016-06-25T20:50:37Z

src/elements/Header/_Header.js

-    useKeyOnly(center, 'center aligned'),
-    useKeyOnly(right, 'right aligned'),
-    useKeyOnly(justified, 'justified'),
+    aligned === 'justified' ? 'justified' : useValueAndKey(aligned, 'aligned'),


Update to the latest master and replace this with:

-aligned === 'justified' ? 'justified' : useValueAndKey(aligned, 'aligned'), +useAlignedProp(aligned)

now using common.implementsAlignedProp(_Header) and useAlignedProp(aligned)

Docs updated to reflect Tests updated to reflect

levithomason · 2016-06-26T19:36:47Z

test/specs/commonTests.js

@@ -385,6 +388,9 @@ export const implementsAlignedProp = (Component, requiredProps = {}) => {
        it('adds "justified" without "aligned" to className', () => {
          shallow(<Component { ...requiredProps } aligned='justified' />)
            .should.have.className('justified')
+
+          shallow(<Component { ...requiredProps } aligned='justified' />)
+            .should.not.have.className('aligned')


Nice catch. Let's reuse use the chai and chain instead of re-rendering though:

shallow(<Component { ...requiredProps } aligned='justified' />) .should.have.className('justified') .and.should.have.className('aligned')

^ this is a pretty minor win, I'm fine merging this as is.

levithomason · 2016-06-26T19:40:30Z

There are some tweaks / clean up that could be made but I'd like to merge this and do that later. Did you have anything else you wanted to do on this PR?

levithomason · 2016-06-26T20:51:47Z

In the interest of keeping pace, I'm gonna merge this and we can make any further changes on subsequent PR. Thanks for the continued contributions and awesome work!

Update Header to v1 API

9f249cf

- Updates the Header component to v1 Api - Examples updated to match Semantic Docs - Multiple tests for Header variants added to suite

jhchill666 changed the title ~~Update Header to v1 API~~ Header Component updated to v1 API Jun 24, 2016

fixes lint errors

4fdc34d

fixes lint errors

4317290

levithomason reviewed Jun 24, 2016
View reviewed changes

jhchill666 added 2 commits June 24, 2016 17:45

Redundant SubheaderComponent element removed in favour of tag inline

bdb958d

Redundant HeaderComponent element removed in favour of tag inline

d10fcda

levithomason reviewed Jun 24, 2016
View reviewed changes

useKeyOrValueAndKey implemented to allow key or key and value denomin…

a69c39e

…ation for attached prop

levithomason reviewed Jun 24, 2016
View reviewed changes

This was referenced Jun 24, 2016

Update legacy component APIs to the v1 API #269

Closed

Better document component className buildup methods #282

Closed

left|center|right|justified props replaced with aligned prop

100525c

tests updated to reflect examples updated to reflect

levithomason reviewed Jun 25, 2016
View reviewed changes

jhchill666 added 6 commits June 26, 2016 06:56

Header Subheader and headings tests simplified to just use isComformant

6a6d87a

Image Component updated to v1 API

ad81023

Image Component updated to v1 API

6330ca0

Merge branch 'master' into feature/header

e5d4be8

Header updated to use _.meta props correctly

2508c5f

Docs updated to reflect Tests updated to reflect

Updated Icon/Image tests for Header components

2efe6ec

jhchill666 added 2 commits June 26, 2016 09:38

Adds new commonTest method for Icon/Image assertions

7ea6275

Fixes lint error

20bf1ad

levithomason reviewed Jun 26, 2016
View reviewed changes

levithomason merged commit 4876cff into Semantic-Org:master Jun 26, 2016

levithomason added the enhancement label Jul 5, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Header Component updated to v1 API #278

Header Component updated to v1 API #278

jhchill666 commented Jun 24, 2016

codecov-io commented Jun 24, 2016 •

edited

Loading

levithomason Jun 24, 2016

jhchill666 Jun 24, 2016

levithomason Jun 24, 2016

jhchill666 Jun 26, 2016

levithomason Jun 24, 2016

jhchill666 Jun 24, 2016 •

edited

Loading

levithomason Jun 24, 2016

levithomason Jun 24, 2016

levithomason Jun 25, 2016

jhchill666 Jun 26, 2016

levithomason Jun 26, 2016

levithomason Jun 26, 2016

levithomason commented Jun 26, 2016

levithomason commented Jun 26, 2016

Header Component updated to v1 API #278

Header Component updated to v1 API #278

Conversation

jhchill666 commented Jun 24, 2016

codecov-io commented Jun 24, 2016 • edited Loading

Current coverage is 87.76%

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

jhchill666 Jun 24, 2016 • edited Loading

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

levithomason commented Jun 26, 2016

levithomason commented Jun 26, 2016

codecov-io commented Jun 24, 2016 •

edited

Loading

jhchill666 Jun 24, 2016 •

edited

Loading