fix(data-list): center align the drag icon within the row of core example#7582
fix(data-list): center align the drag icon within the row of core example#7582sg00dwin wants to merge 2 commits intopatternfly:mainfrom
Conversation
|
Preview: https://patternfly-pr-7582.surge.sh A11y report: https://patternfly-pr-7582-a11y.surge.sh |
srambach
left a comment
srambach
left a comment
There was a problem hiding this comment.
These changes look good to me. A couple of notes -
- @mcoker does this qualify as a breaking change? We've added
pf-v6-c-buttonandpf-m-plainwhich I think would be ok? But also removedpf-v6-c-data-list__item-draggable-icon - if removing
pf-v6-c-data-list__item-draggable-iconis ok, then I think we can remove the styling for it (just icon color) and remove it from the docs. Otherwise, I guess we need to add that back onto the icon span?
There was a problem hiding this comment.
Looks good, just a couple of updates please. I left a comment about removing the disabled attribute from an hbs block - we should also replace disabled with data-list-item-draggable-button--IsDisabled=true on this line, too
Will answer @srambach's comments in another comment
| button--IsBlock='pf-m-block' | ||
| button--IsSettings='pf-m-settings' | ||
| button--IsHamburger='pf-m-hamburger' | ||
| button--IsDraggable='pf-v6-c-data-list__item-draggable-button' |
There was a problem hiding this comment.
This this is a class specific to data-list, can you take this out and move it to data-list-item-draggable-button?
I would just remove this line, then in data-list-item-draggable-button.hbs, just pass button--modifier=(concat pfv "data-list__item-draggable-button"). My concat syntax may be wrong 😅 Just want to get the hard-coded pf-v6-c- part out of the string.
| {{#> data-list-item-row}} | ||
| {{#> data-list-item-control}} | ||
| {{> data-list-item-draggable-button data-list-item-draggable-button--modifier="pf-m-disabled" data-list-item-draggable-button--attribute=(concat 'id="' data-list--id '-draggable-button-1" aria-describedby="draggable-help" aria-labelledby="' data-list--id '-draggable-button-1 ' data-list--id '-item-1" disabled')}} | ||
| {{> data-list-item-draggable-button data-list-item-draggable-button--IsDisabled=true data-list-item-draggable-button--attribute=(concat 'id="' data-list--id '-draggable-button-1" aria-describedby="draggable-help" aria-labelledby="' data-list--id '-draggable-button-1 ' data-list--id '-item-1" disabled')}} |
There was a problem hiding this comment.
| {{> data-list-item-draggable-button data-list-item-draggable-button--IsDisabled=true data-list-item-draggable-button--attribute=(concat 'id="' data-list--id '-draggable-button-1" aria-describedby="draggable-help" aria-labelledby="' data-list--id '-draggable-button-1 ' data-list--id '-item-1" disabled')}} | |
| {{> data-list-item-draggable-button data-list-item-draggable-button--IsDisabled=true data-list-item-draggable-button--attribute=(concat 'id="' data-list--id '-draggable-button-1" aria-describedby="draggable-help" aria-labelledby="' data-list--id '-draggable-button-1 ' data-list--id '-item-1"')}} |
Just a nit, but you can remove the disabled attr here now that you're passing button--IsDisabled to the button. That will add the disabled attr
patternfly/src/patternfly/components/Button/button.hbs
Lines 73 to 77 in cb3fa15
|
@srambach good catches!
The button classes are in the react component, so IMO that part of the update just fixes a bug in the core docs and shouldn't be breaking. However we do need to add I think that update would be this:
Yep! Need to add it back. |
|
@mcoker Addressed comments
|
|
This PR has been automatically marked as stale because it has not had activity in the last 60 days. |
4 participants
fixes #7579
https://patternfly-pr-7582.surge.sh/components/data-list#draggable