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.

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

InlineBlockList component #136

Merged
merged 3 commits into from
Mar 23, 2017
Merged

InlineBlockList component #136

merged 3 commits into from
Mar 23, 2017

Conversation

mperrotti
Copy link
Contributor

Related issues

Needed for header and footer navigation redesign for replatform

Description

Takes an array of text strings or elements and creates a list that is displayed inline

Screenshots (if applicable)

screen shot 2017-03-20 at 1 46 13 pm
screen shot 2017-03-20 at 1 46 22 pm
screen shot 2017-03-20 at 1 46 44 pm

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 85.577% when pulling 88e8700 on inline_block_list into ede53c5 on master.

}

&:after {
content: attr(data-separator);
Copy link
Contributor

Choose a reason for hiding this comment

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

🕶️

const {
className,
items,
separator,
Copy link
Contributor

Choose a reason for hiding this comment

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

love the flexibility here with separator

return (
<ul
className={classNames}
{...other}>
Copy link
Contributor

Choose a reason for hiding this comment

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

minor code style thing - we've been wrapping the closing > to the next line for readability

() => (
<InfoWrapper>
<InlineBlockList
style={{padding: '20px'}}
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer to import a Section & Chunk here to an inline style. Showing layout components in the examples (even if they're unrelated to InlineBlockList) seems like it would be helpful

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 85.358% when pulling 3153032 on inline_block_list into ede53c5 on master.

@@ -18,7 +18,7 @@ const ITEMS = [
'日本語',
'한국어',
],
SEPARATOR = '·';
SEPARATOR = '';
Copy link
Contributor

Choose a reason for hiding this comment

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

💯 ⛄️

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 85.358% when pulling bc6bcb8 on inline_block_list into ede53c5 on master.

@akdetrick akdetrick merged commit 94043e6 into master Mar 23, 2017
@chenrui333 chenrui333 deleted the inline_block_list branch September 16, 2019 22:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants