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.

Sign up for GitHub

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

Header Component updated to v1 API #278

Merged
merged 15 commits into from
Jun 26, 2016
Merged

Header Component updated to v1 API #278

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
9f249cf
Update Header to v1 API
jhchill666 Jun 24, 2016
4fdc34d
fixes lint errors
jhchill666 Jun 24, 2016
4317290
fixes lint errors
jhchill666 Jun 24, 2016
bdb958d
Redundant SubheaderComponent element removed in favour of tag inline
jhchill666 Jun 24, 2016
d10fcda
Redundant HeaderComponent element removed in favour of tag inline
jhchill666 Jun 24, 2016
a69c39e
useKeyOrValueAndKey implemented to allow key or key and value denomin…
jhchill666 Jun 24, 2016
100525c
left|center|right|justified props replaced with aligned prop
jhchill666 Jun 25, 2016
6a6d87a
Header Subheader and headings tests simplified to just use isComformant
jhchill666 Jun 26, 2016
ad81023
Image Component updated to v1 API
jhchill666 Jun 24, 2016
6330ca0
Image Component updated to v1 API
jhchill666 Jun 24, 2016
e5d4be8
Merge branch 'master' into feature/header
jhchill666 Jun 26, 2016
2508c5f
Header updated to use _.meta props correctly
jhchill666 Jun 26, 2016
2efe6ec
Updated Icon/Image tests for Header components
jhchill666 Jun 26, 2016
7ea6275
Adds new commonTest method for Icon/Image assertions
jhchill666 Jun 26, 2016
20bf1ad
Fixes lint error
jhchill666 Jun 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions docs/app/Examples/elements/Header/Variations/HeaderTextAlignmentExample.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ export default class HeaderTextAlignmentExample extends Component {
render() {
return (
<Segment>
<Header.H3 className='right aligned'>
<Header.H3 aligned='right'>
Float Right
</Header.H3>
<Header.H3 className='left aligned'>
<Header.H3 aligned='left'>
Float Left
</Header.H3>
<Header.H3 className='justified'>
<Header.H3 aligned='justified'>
This text takes up the full width of the container
</Header.H3>
<Header.H3 className='center aligned'>
<Header.H3 aligned='center'>
Centered
</Header.H3>
</Segment>
Expand Down
23 changes: 7 additions & 16 deletions src/elements/Header/_Header.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,13 @@ import {
function _Header(props) {
const {
_sdClass, _headerElement,
left, center, right, justified, dividing, block, attached, floating,
aligned, dividing, block, attached, floating,
icon, image, children, className,
} = props

const classes = cx(
Copy link
Member

Choose a reason for hiding this comment

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

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
)

Copy link
Contributor Author

@jhchill666 jhchill666 Jun 24, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

_sdClass, 'ui',
useKeyOnly(left, 'left aligned'),
useKeyOnly(center, 'center aligned'),
useKeyOnly(right, 'right aligned'),
useKeyOnly(justified, 'justified'),
aligned === 'justified' ? 'justified' : useValueAndKey(aligned, 'aligned'),
Copy link
Member

Choose a reason for hiding this comment

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

Update to the latest master and replace this with:

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

useKeyOnly(dividing, 'dividing'),
useKeyOnly(block, 'block'),
useKeyOrValueAndKey(attached, 'attached'),
Expand All @@ -47,6 +44,9 @@ _Header._meta = {
library: META.library.semanticUI,
name: '_Header',
type: META.type.element,
props: {
aligned: ['left', 'center', 'right', 'justified'],
},
}

_Header.propTypes = {
Expand All @@ -67,17 +67,8 @@ _Header.propTypes = {
PropTypes.element,
]),

/** Align header content to left */
left: PropTypes.bool,

/** Align header content to center */
center: PropTypes.bool,

/** Align header content to right */
right: PropTypes.bool,

/** Justify content to available space */
justified: PropTypes.bool,
/** Align header content */
aligned: PropTypes.oneOf(_Header._meta.props.aligned),

/** Divide header from the content below it */
dividing: PropTypes.bool,
Expand Down
12 changes: 8 additions & 4 deletions test/specs/elements/Header/_Header-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import React from 'react'

import _Header from 'src/elements/Header/_Header'
import * as common from 'test/specs/commonTests'

describe('_Header', () => {
common.propKeyOnlyToClassName(_Header, 'left', { className: 'aligned' })
common.propKeyOnlyToClassName(_Header, 'center', { className: 'aligned' })
common.propKeyOnlyToClassName(_Header, 'right', { className: 'aligned' })
common.propKeyOnlyToClassName(_Header, 'justified')
common.propKeyAndValueToClassName(_Header, 'aligned')
common.propKeyOnlyToClassName(_Header, 'dividing')
common.propKeyOnlyToClassName(_Header, 'block')
common.propKeyOnlyToClassName(_Header, 'attached')
common.propKeyOnlyToClassName(_Header, 'floating')

it('does not have aligned class when justified', () => {
shallow(<_Header aligned='justified' />)
.should.not.have.className('aligned')
})
Copy link
Member

Choose a reason for hiding this comment

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

Once updated and the new prop util and common test are used, you can delete this test:

-
-  it('does not have aligned class when justified', () => {
-    shallow(<_Header aligned='justified' />)
-      .should.not.have.className('aligned')
-  })

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

})