#261: Implementation of Header Component #284
Conversation
|
|
||
| export type DictionaryHeaderProps = { | ||
| type DictionaryHeaderProps = { |
There was a problem hiding this comment.
Please export component props
| <span css={descriptionStyle(theme)}>{textToShow}</span> | ||
| {needsToggle && ( | ||
| <span css={linkStyle(theme)} onClick={() => setIsExpanded((prev) => !prev)}> | ||
| {isExpanded ? ' Read less' : ' Show more'} |
There was a problem hiding this comment.
Why the empty space preceding the text?
Would you consider instead {' '}{isExpanded ? 'Read less' : 'Show more'}?
There was a problem hiding this comment.
Good point, didn't think to do that
| justify-content: center; | ||
| `; | ||
|
|
||
| // These constants can be adjusted based on design requirements |
There was a problem hiding this comment.
All constants can be adjusted ;)
Can we change this comment to instead explain what this threshold is used for and/or why it is set to this default?
I would suggest renaming to DESCRIPTION_LENGTH_THRESHOLD. Specificity + comment description is helpful.
| const needsToggle = description && description.length > DESCRIPTION_LENGTH_THRESHOLD; | ||
| // We want to show all the text if it is not long or if it is already expanded via state variable | ||
| const showFull = isExpanded || !needsToggle; | ||
| // Based off of showFull, we determine the text to show, either its the full description or a truncated version | ||
| const textToShow = showFull ? description : description.slice(0, DESCRIPTION_LENGTH_THRESHOLD) + '... '; |
There was a problem hiding this comment.
Was not too happy with this implementation, I have refactored this into something that appropriately implements this to spec in another branch
Summary
Implement dictionary header component, based off of the theming as well as adding in tests to exercise edge cases
Related Issue
Description of Changes
UI
DictionaryHeaderwhich displays current dictionary name, version and descriptionReadiness Checklist
.env.schemafile and documented in the README