Refactor AttributeField into sub-components #35997
Conversation
mattsherman
added
focus: variation
github-actions
bot
added
focus: react admin
package: @woocommerce/components
Test Results SummaryCommit SHA: 126d401
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
mattsherman
changed the title
There was a problem hiding this comment.
Love the refactor direction here and this is testing well (or at least as well as trunk) for me.
I left one comment on the "chips" but not absolutely necessary to resolve here since this issue already existed prior to this PR.
On both this branch and trunk I'm having issues whenever I reorder or make changes to an existing attribute. All attributes seem to inherit the modified attribute properties.
Pre-approving this PR since this issue also exists for me on trunk but it would be great if @louwie17 could give this a quick spin and share any pointers around the above issue if it can be reproduced.
| onDragStart, | ||
| onDragEnd, | ||
| }: ListItemProps ) => { | ||
| const isDraggable = onDragEnd && onDragStart; | ||
|
|
||
| return ( | ||
| <div className={ classnames( 'woocommerce-list-item' ) }> | ||
| <div className={ classnames( 'woocommerce-list-item', className ) }> |
| <div>{ attribute.name }</div> | ||
| <div className="woocommerce-attribute-list-item__options"> | ||
| { attribute.options.slice( 0, 2 ).map( ( option, index ) => ( | ||
| <div |
There was a problem hiding this comment.
Just noting that these could be updated to the Tag component, but not absolutely necessary in this PR given this was pre-existing.
There was a problem hiding this comment.
Good idea. I'll save that for a follow up PR since this one is approved, has no conflicts, and all tests are passing!
| @@ -40,6 +55,27 @@ type AttributeForm = { | |||
| }; | |||
|
|
|||
| export const AddAttributeModal: React.FC< AddAttributeModalProps > = ( { | |||
| title = __( 'Add attributes', 'woocommerce' ), | |||
There was a problem hiding this comment.
I like the label props. Very flexible.
All Submissions:
Changes proposed in this Pull Request:
This PR refactors
AttributeFieldinto sub-components, where those sub-components expose all UI strings as props, allowing them to be overridden as necessary. This will make creating anOptionsField(or, allowingAttributeFieldto switch between Attributes and Options) easier.Part of #35770.
How to test the changes in this Pull Request:
Other information:
pnpm --filter=<project> changelog add?FOR PR REVIEWER ONLY: