[terra-folder-tree] Base Component Creation #1888
Conversation
…e context of embedded iframe content (#1804) Co-authored-by: Vinay Bhargav Arni Ragunathan <[email protected]> Co-authored-by: Saad Adnan <[email protected]>
| /** | ||
| * The heading level for the title of the folder tree. | ||
| */ | ||
| headerLevel: PropTypes.number, |
There was a problem hiding this comment.
Is this a value that we want to be configurable or should it be a constant value for all FolderTree components.
There was a problem hiding this comment.
Talked with @eawww, we want to keep the header level configurable since we don't know where on a page FolderTree will be used so consumers will need the option to set custom heading levels.
| onClick={onClick} | ||
| /> | ||
| {/* eslint-disable-next-line react/forbid-dom-props */} | ||
| <span style={{ paddingLeft: `${level}rem` }}> |
There was a problem hiding this comment.
Why are we adding a span element around the Terra Arrange? Are we trying to use this like the Spacer component?
| <span style={{ paddingLeft: `${level}rem` }}> | ||
| <Arrange | ||
| fitStart={( | ||
| <Spacer paddingLeft="medium" paddingRight="medium" isInlineBlock> |
There was a problem hiding this comment.
It looks like we are using both a span and Spacer element to add padding.
| display: flex; | ||
| margin: 0; | ||
| min-height: var(--terra-folder-tree-item-min-height, 2.92857rem); | ||
| overflow-wrap: anywhere; |
There was a problem hiding this comment.
We would want this to be "break-word". I don't think we want to use anywhere as our option.
https://developer.mozilla.org/en-US/docs/Web/CSS/overflow-wrap
There was a problem hiding this comment.
With break-word, long strings without spaces do not wrap which is why I used anywhere, do you know of a way to handle that with break-word?
| :local { | ||
| .folder-tree-item { | ||
| align-items: center; | ||
| background-color: var(--terra-folder-tree-item-background-color, #fff); |
There was a problem hiding this comment.
I think it is better to use background instead of background-color. The background style would allow for both colors and gradients without a future style change.
| .selected { | ||
| background-color: var(--terra-folder-tree-item-selected-background-color, darken(#ebf6fd, 2%)); | ||
| } | ||
|
|
There was a problem hiding this comment.
What about the focus state?
There was a problem hiding this comment.
The focus state should be added with keyboard navigation (UXPLATFORM-9776).
| :local { | ||
| .orion-fusion-theme { | ||
| --terra-folder-tree-item-background-color-hover: #f4fafe; | ||
| // --terra-folder-tree-item-background-color-selected: ; |
There was a problem hiding this comment.
Looks like you might need to remove this commented code.
| <Arrange | ||
| fitStart={( | ||
| <Spacer paddingLeft="medium" paddingRight="medium" isInlineBlock> | ||
| {itemIcon} | ||
| </Spacer> | ||
| )} | ||
| fill={label} | ||
| alignFitStart="center" | ||
| /> | ||
| </span> |
There was a problem hiding this comment.
Instead of adding this complexity. Can't we just use the listStyleImage property to set the image? Also, are we able to leverage list-style-position to assist with the indentation? Can we maybe limit the custom styling necessary?
| aria-selected={isSelected} | ||
| > | ||
| <input | ||
| type="radio" |
There was a problem hiding this comment.
These shouldn't be radio buttons since they can't be properly associated with each other and we'll be using aria-selected instead.
| text={title} | ||
| level={headerLevel} | ||
| /> | ||
| <ul |
There was a problem hiding this comment.
This needs an aria-labeledby pointing to the header above it or an aria-label with the same text.
packages/terra-folder-tree/src/subcomponents/FolderTreeItem.jsx
Outdated
Show resolved
Hide resolved
packages/terra-folder-tree/src/subcomponents/FolderTreeItem.jsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Eric Wilson <[email protected]>
Co-authored-by: Eric Wilson <[email protected]>
Summary
The PR adds the terra-folder-tree base component, which displays items in a hierarchical folder structure. Keyboard navigation and expand/collapse functionality are not included in this PR and will be implemented as part of a future Jira.
Testing
This change was tested using:
Reviews
In addition to engineering reviews, this PR needs:
Additional Details
This PR resolves:
UXPLATFORM-9679
Thank you for contributing to Terra.
@cerner/terra