Add tag sort, filter, search and pagination #3660
Conversation
|
Blocked on #3659 landing first. |
| <div className="p-segment-toggle"> | ||
| {options.map(({ title, value }) => ( | ||
| <Button | ||
| className={classNames("p-segment-toggle__button", { | ||
| "is-active": value === selected, | ||
| })} | ||
| key={title} | ||
| onClick={() => onSelect(value)} | ||
| > | ||
| {title} | ||
| </Button> | ||
| ))} | ||
| </div> |
There was a problem hiding this comment.
We already have a Tabs component in Vanilla that seems to do the same thing https://vanillaframework.io/docs/patterns/tabs#default
Could you use the vanilla copmonent instead and follow accessibility recommendations (add roles tablist, tab, aria-selected to the currently active tab).
More on this here:
https://vanillaframework.io/docs/patterns/tabs#accessibility
There was a problem hiding this comment.
Oh nice, thanks I didn't know about that one. I think while the styling is correct (I'll update this component to use those classes) I'm not sure those aria roles are right as it's not changing tabs, it's filtering the contents of the table.
It seems like role="radiogroup" might be the right option here. I've updated the component. What do you think?
There was a problem hiding this comment.
Sure, that's even better. Thanks!
| expect(screen.getByTestId(TestId.TagListControls)).toBeInTheDocument(); | ||
| expect(screen.getByTestId(TestId.TagTable)).toBeInTheDocument(); |
There was a problem hiding this comment.
Once you use semantic HTML markup, using a test id will be no longer necessary. Please use a combination of semantic role and label instead.
| <div className="kvm-action-bar"> | ||
| <div className="kvm-action-bar__actions">{actions}</div> | ||
| <div className="kvm-action-bar__search"> | ||
| <div className="action-bar" {...props}> |
There was a problem hiding this comment.
| <div className="action-bar" {...props}> | |
| <div className="action-bar" role="group" {...props}> |
| /> | ||
| <> | ||
| <TagListControls | ||
| data-testid="TagListControls" |
There was a problem hiding this comment.
| data-testid="TagListControls" | |
| aria-label="tag list controls" |
| tagCount={tags.length} | ||
| /> | ||
| <TagTable | ||
| data-testid="TagTable" |
There was a problem hiding this comment.
| data-testid="TagTable" | |
| aria-label="tags" |
Add the action bar to Tags to sort, filter and search. Add pagination to the table. Fixes: canonical/app-tribe#703. Fixes: canonical/app-tribe#698. Fixes: canonical/app-tribe#697. Fixes: canonical/app-tribe#696.
huwshimi
force-pushed
the
tags-table-filter
branch
from
ff8266c to
74e5b16
Compare
Done
QA
MAAS deployment
To run this branch you will need access to one of the following MAAS deployments:
Running the branch
You can run this branch by:
QA steps
TAGS_PER_PAGEvalue to less than the number of tags.Fixes
Fixes: https://github.com/canonical-web-and-design/app-tribe/issues/703.
Fixes: https://github.com/canonical-web-and-design/app-tribe/issues/698.
Fixes: https://github.com/canonical-web-and-design/app-tribe/issues/697.
Fixes: https://github.com/canonical-web-and-design/app-tribe/issues/696.
Screenshots