-
Notifications
You must be signed in to change notification settings - Fork 55
feat(Tree): add behaviors with list/listitem roles #1928
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1928 +/- ##
=========================================
+ Coverage 70.39% 70.4% +<.01%
=========================================
Files 889 892 +3
Lines 7851 7867 +16
Branches 2264 2271 +7
=========================================
+ Hits 5527 5539 +12
- Misses 2313 2315 +2
- Partials 11 13 +2
Continue to review full report at Codecov.
|
} | ||
|
||
export type TreeItemBehaviorProps = { | ||
/** Indicated if tree title has a subtree */ |
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.
/** Indicated if tree title has a subtree */ | |
/** Indicates whether `TreeTitle` has a subtree */ |
@@ -20,7 +20,7 @@ const treeItemBehavior: Accessibility<TreeItemBehaviorProps> = props => ({ | |||
root: { | |||
role: 'none', | |||
...(props.hasSubtree && { | |||
'aria-expanded': props.open, | |||
'aria-expanded': props.open ? 'true' : '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.
There are a few instances in Stardust where we pass a boolean
to aria-expanded
. Let's report it as an issue?
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.
hmm let me take a look
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.
for consistency I made this one boolean as well. what do you think?
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.
Looks good to me
export default treeTitleAsListItemTitleBehavior | ||
|
||
type TreeTitleBehavior = { | ||
/** Indicated if tree title has a subtree */ |
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.
/** Indicated if tree title has a subtree */ | |
/** Indicates whether `TreeTitle` has a subtree */ |
|
||
describe('TreeItemAsListItemBehavior', () => { | ||
describe('role', () => { | ||
test(`is 'none' if not a leaf`, () => { |
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.
How about adding a test where its a leaf?
expect(expectedResult.attributes.root.role).toEqual('treeitem') | ||
}) | ||
|
||
test(`is 'treeitem' if not a leaf`, () => { |
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.
test(`is 'treeitem' if not a leaf`, () => { | |
test(`is 'treeitem' if a leaf`, () => { |
}) | ||
} | ||
|
||
type TreeBehaviorProps = {} & Pick<AccessibilityAttributes, 'aria-labelledby'> |
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.
type TreeBehaviorProps = {} & Pick<AccessibilityAttributes, 'aria-labelledby'> | |
type TreeBehaviorProps = Pick<AccessibilityAttributes, 'aria-labelledby'> |
nit, currently looks a bit strange
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.
Looks good to me and I think it is as good as it currently can be.
Can you please add an example? It's quite complex to put all the behaviors on the right place
…rdust-ui/react into chore/tree-as-list-behaviors
pls add changelog before merging |
Needed in order to make VO navigation possible. It will read the tree as a list, but it will calculate the list item accordingly, based on the level, setsize, posinset.
Feedback on how to name these behaviors?
Also added missing tests for tree behaviors.