-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Segment Examples #113
Segment Examples #113
Conversation
invariant( | ||
!_.any( Children.map(children, child => child.type !== Segment) ), | ||
'May only contain children of type Segment.', | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@levithomason this would be our first use of fbjs
to throw a possible invariant
violation in this repo. The intent is to help prevent misuse of the components. Still need to add support for nested groups. (Note: the second argument to invariant
is optional and would shed some weight during production builds if omitted.)
What's weird though is when I tried to create a simple invariant
for Children(count) >= 2
- I ran into issues with conformance tests.
If we like the pattern we could also consider adding a runtime check for number of children, though that's something we may want to pair on given my failed attempts.
Alternatively we could try an approach more like this perhaps?
static propTypes = {
children: PropTypes.oneOfType([
PropTypes.oneOfType([
PropTypes.instanceOf(Segment),
PropTypes.instanceOf(this),
]),
PropTypes.arrayOf(PropTypes.oneOfType([
PropTypes.instanceOf(Segment),
PropTypes.instanceOf(this),
])),
]).isRequired,
className: PropTypes.string,
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In attempting to allow Segments
to support nested Segments
I tried the propTypes approach, as well as the following without luck:
invariant(
!_.any(Children.map(children, child => {
return _.includes( ['Segment', 'Segments'], _.get(child, 'type._meta.name') );
})),
'May only contain children of type Segment or Segments.',
);
As a result, I'm going to backpedal to children: PropTypes.node
for now, and remove the fbjs
and invariant
calls. The net result is the component will accept any children until we're able to figure out a method which works.
mutuallyExclusive: exclusives => { | ||
return (props, propName, componentName) => { | ||
_.forEach(exclusives, exclusiveProp => { | ||
if (props[propName] && props[exclusiveProp]) { | ||
throw new Error( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Per Reusable Components doc we should not throw, but return
errors from custom validators.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch, thanks!
> | ||
<Message className='info'> | ||
Attached segments are designed to be used with other <code>attached</code> variations like | ||
the <a href='#Header-Variations-HeaderAttachedExample'>attached header</a> or <i>attached messages</i>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for the anchor link here!
import React, {Component} from 'react'; | ||
import {Segment, Segments} from 'stardust'; | ||
|
||
export default class SegmentCompactGroupsExample extends Component { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another tiny difference between file & class name, just a bonus 's' 😉
ee66f1f
to
36956a6
Compare
All review feedback incorporated. While I was in there I also refactored |
Adds Semantic UI
Segment
examples to the Stardust repo. Also creates aSegments
component which works in concert withSegment
to provide grouping behavior.Segments
component is considered a child ofSegment
from a documentation purpose and so it does not appear in the list of components, but will appear in the examples.Resolves #112 and closes #114. Discovers #116 and #120.