-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
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.
Awesome component composition thinking here.
title: 'This is a full-length title', | ||
path: 'url', | ||
description: "Here's a reasonable length for a description.", | ||
level: <LevelIndicator level={2} />, |
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 is one possibility for the interface; another would be to pass the level number directly. Curious why you chose to pass a React element instead of the level number? Do you think we may want to pass some other element in the future?
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.
Yea it's probably safe to just pass the level number.
let squareColor = ''; | ||
num > this.props.level | ||
? (squareColor = 'gray-light') | ||
: (squareColor = levelColor); |
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 could be combined into one expression:
const squareColor = num > props.level ? 'gray-light' : levelColor;
className={`inline-block w6 bg-${squareColor} align-middle mr3`} | ||
/> | ||
); | ||
}); |
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 is a great use case for .map()
! When you're taking one array (which should probably be Object.keys(levels)
?) and transforming every item into another corresponding item in a new array (e.g. a React element), .map()
is what you're looking for.
This PR does two things:
1️⃣ Adds a new
LevelIndicator
component based on the work done in /help (ty @danswick).2️⃣ Adds optional
level
andlanguage
props toCard
(to be used for /help tutorials).Closes #53