chore(table): update dropdown to button#6329
chore(table): update dropdown to button#6329mattnolting wants to merge 3 commits intopatternfly:v6from
Conversation
|
Preview: https://patternfly-pr-6329.surge.sh A11y report: https://patternfly-pr-6329-a11y.surge.sh |
bd0ad52 to
4ad0d17
Compare
| {{#if table-td--toggle}} | ||
| <button class="{{pfv}}button pf-m-plain{{#if table-tr--expanded}} pf-m-expanded{{/if}}" {{#if table-td--button--attribute}}{{{table-td--button--attribute}}}{{/if}} {{#if table-tr--expanded}}aria-expanded="true"{{/if}}> | ||
| {{#if table-td--action}} | ||
| {{#> button button--IsSmall=table--IsCompact button--IsPlain=true button--id=(concat table--id '-action-kebab-' table-tr--index) button--aria-label="Actions"}} |
There was a problem hiding this comment.
Looks like table--IsCompact is used just for adjusting the button size. Should it also be used to add the pf-m-compact modifier?
| {{#if table-td--toggle}} | ||
| <button class="{{pfv}}button pf-m-plain{{#if table-tr--expanded}} pf-m-expanded{{/if}}" {{#if table-td--button--attribute}}{{{table-td--button--attribute}}}{{/if}} {{#if table-tr--expanded}}aria-expanded="true"{{/if}}> | ||
| {{#if table-td--action}} | ||
| {{#> button button--IsSmall=table--IsCompact button--IsPlain=true button--id=(concat table--id '-action-kebab-' table-tr--index) button--aria-label="Actions"}} |
There was a problem hiding this comment.
It looks like table-IsCompact is being used just to trigger a small button. Should it also be used to add the pf-m-compact modifier to the table?
Also, I'm wondering if it makes sense to make the button be a menu-toggle instead. Maybe not, since an action could be anything, but here it's used as a toggle for a menu, which makes it seem like it should be a menu-toggle then.
I think a kebab menu is probably the most frequent use case, so it's good to show that but at the same time, it's not the most basic example either. Curious what @mcoker thinks before changing anything. This could also definitely wait for a later day.
There was a problem hiding this comment.
LMK if any of my comments are planned to be handled by another PR.
The overflow examples end up a bit visually broken:
Since the overflow menu is wrapped in a td with table-td--action="true"
Additionally expanded examples need to be updated to use table-td--IsToggle instead of table-td--toggle
| {{#> table-td table-td--action="true"}} | ||
| {{> table--overflow-menu}} | ||
| {{/table-td}} | ||
| {{#> table-td table-td--action="true"}}{{/table-td}} |
There was a problem hiding this comment.
Is the plan to add content between these tags, or for tabe-td--action to handle rendering whatever content? If the latter, could these be written without the hashtag, {{> table-td table-td--action="true"}}?
There was a problem hiding this comment.
Yep, sure could. Also don't need quotes for booleans {{> table-td table-td--action=true}}
145d027 to
3e09a1d
Compare
3e09a1d to
dddccaa
Compare
| {{~#if table-td--check}} {{pfv}}table__check{{/if}} | ||
| {{~#if table-td--action}} {{pfv}}table__action{{/if}} | ||
| {{~#if table-td--IsAction}} {{pfv}}table__action{{/if}} | ||
| {{~#if table-td--HasOverflowMenu}} {{pfv}}table__action{{/if}} |
There was a problem hiding this comment.
This probably needs to be updated to table-td--UsesOverflowMenu as that is being used elsewhere. Currently nothing is being rendered in the last column of the overflow table examples.
| {{/table-td}} | ||
| {{#> table-td table-td--action="true"}} | ||
| {{#> table-td table-td--IsAction=true}} | ||
| {{> dropdown dropdown--id=(concat table--id inline-edit--row '-dropdown-kebab') dropdown-toggle--IsPlain="true" dropdown-menu--modifier="pf-m-align-right"}} |
There was a problem hiding this comment.
This dropdown and line 395 just need to be removed.
| {{/table-td}} | ||
| {{#> table-td table-td--action="true"}} | ||
| {{#> table-td table-td--IsAction=true}} | ||
| {{> dropdown dropdown--id=(concat card--id "-dropdown-kebab-1") dropdown-menu--modifier="pf-m-align-right" dropdown-toggle--IsPlain="true"}} |
There was a problem hiding this comment.
Similar to above for removing these dropdowns in this file (also lines 91, 122 and 153)
| {{#> button button--modifier="pf-m-plain" button--attribute='aria-label="Remove"'}} | ||
| <i class="fas fa-ellipsis-h" aria-hidden="true"></i> | ||
| {{/button}} |
There was a problem hiding this comment.
These buttons with the horizontal ellipsis could probably be removed as well
| {{/table-td}} | ||
| {{#> table-td table-td--action="true"}} | ||
| {{#> table-td table-td--IsAction=true}} | ||
| {{> tabs--table-overflow-menu tabs--table-overflow-menu--id=(concat table--id "-dropdown-kebab-1")}} |
There was a problem hiding this comment.
Similar to above this tabs--table-overflow-menu can be removed from the td's in this demo.
|
Per @mattnolting closing this PR, we'll update the dropdowns in a follow-up PR |
applies to #6294
This PR updates table dropdowns to buttons in advance of updated padding/positioning values