-
Notifications
You must be signed in to change notification settings - Fork 55
feat(ItemLayout): added ItemLayout component #60
Conversation
Codecov Report
@@ Coverage Diff @@
## master #60 +/- ##
==========================================
+ Coverage 86.29% 87.02% +0.72%
==========================================
Files 39 41 +2
Lines 664 709 +45
Branches 90 94 +4
==========================================
+ Hits 573 617 +44
- Misses 88 89 +1
Partials 3 3
Continue to review full report at Codecov.
|
header="Irving Kuhic" | ||
headerMedia="7:26:56 AM" | ||
content="Program the sensor to the SAS alarm through the haptic SQL card!" | ||
debug={knobs.debug} |
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.
Since this is the full example with a debug display knob to help the user understand the full layout, let's also include some endMedia
here. Perhaps some kind of icon would be great.
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.
I added the end media with ellipses (…) to this examples, as well as the truncate example, as we have there even more knobs.
|
||
handleMouseLeave = () => { | ||
this.setState({ isHovering: false }) | ||
} |
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.
The ItemLayout should not have any listeners or state. Let's strip this.
A layout component should only handle the CSS layout itself. The rest of the logic is left to the component implementing the layout.
Please add https://help.github.com/articles/closing-issues-using-keywords/ |
-updated ListItem component to work with the ItemLayout
# Conflicts: # CHANGELOG.md # src/components/ItemLayout/itemLayoutVariables.ts # src/components/List/List.tsx # src/components/List/ListItem.tsx # src/themes/teams/components/List/listItemStyles.ts
|
||
// Item | ||
vars.itemPaddingLeft = pxToRem(20) | ||
vars.itemPaddingRight = pxToRem(18) |
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.
This looks like a regression. The listVariables.ts was moved to listItemVariables.ts and the variable names were updated to remove the list*
portion.
color: '#fff', | ||
cursor: 'pointer', | ||
}, | ||
}), |
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.
We should move the non-position related styles back to the ListItem from the Layout. The layout should not define colors, fonts, etc but only layout styling.
}, | ||
|
||
media: ({ props }) => { | ||
const { important } = props |
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.
Since important
will likely vary depending on the parent component implementing the items, let's move this prop and style also back to the ListItem side. Keeping just position and DOM structure in the layouts.
lineHeight: variables.contentLineHeight, | ||
}), | ||
contentMedia: () => ({}), | ||
endMedia: () => ({}), |
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.
No need for empty styles
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.
See comments for changes. Probably good to merge once those are addressed. We can iterate on the rest afterward in other PRs.
# Conflicts: # CHANGELOG.md # src/components/List/ListItem.tsx # src/themes/teams/components/List/listItemStyles.ts
-removed important prop from ItemLayout
# Conflicts: # src/components/List/ListItem.tsx
ItemLayout
The Item layout will be a layout component for aligning content in form of media, header (and headerMedia), content (and contentMedia) and end media. This PR fixes #27 and fixes #71.
With this PR the ListItem is also updated to use the ItemLayout component.
TODO
API Proposal
content: PropTypes.any
Shorthand for primary content.
contentMedia: PropTypes.any
End media for the content
header: PropTypes.any
Shorthand for the header.
headerMedia: PropTypes.any
End media for the header
media: PropTypes.any
General start media.
endMedia: PropTypes.any
General end media
renderContentArea: PropTypes.any
Custom function for generating the content area.
renderHeaderArea: PropTypes.any
Custom functino for generating the header area.
renderMainArea: PropTypes.any
Custom function for generating the main area.
important: PropTypes.bool
An item can appear more important and draw the user's attention.
truncateContent: PropTypes.bool
Prop for indicating if the content should be truncated.
truncateHeader: PropTypes.bool
Prop for indicating if the header should be truncated.
debug: PropTypes.bool
Toggle debug mod
Usage example
Jsx code:
Result: