Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Create tree-control component #36432

Merged
merged 3 commits into from
Jan 19, 2023
Merged

Create tree-control component #36432

merged 3 commits into from
Jan 19, 2023

Conversation

mdperez86
Copy link
Contributor

@mdperez86 mdperez86 commented Jan 13, 2023 •
edited

All Submissions:

Changes proposed in this Pull Request:

Partialy closes #35851

  • This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

  1. Run storybook
  2. Visit http://localhost:6007/?path=/story/woocommerce-admin-experimental-treecontrol--simple-tree
  3. A basic tree control should be shown.
Screen.Recording.2023-01-17.at.11.04.33.AM.mov

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@mdperez86 mdperez86 self-assigned this Jan 13, 2023
@github-actions github-actions bot added the package: @woocommerce/components issues related to @woocommerce/components label Jan 13, 2023
@mdperez86 mdperez86 requested a review from a team January 13, 2023 23:17
@mdperez86 mdperez86 added focus: product management Related to product creation and editing. focus: new product ux revamped product management experience labels Jan 13, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 13, 2023 •
edited

Test Results Summary

Commit SHA: e7f3bbf

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202611m 2s
E2E Tests189006019516m 56s
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.

@mdperez86 mdperez86 mentioned this pull request Jan 14, 2023
Merged
8 tasks
@mdperez86 mdperez86 marked this pull request as ready for review January 17, 2023 13:20
joshuatf
joshuatf approved these changes Jan 18, 2023
View reviewed changes
Copy link
Contributor

@joshuatf joshuatf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful work! Love how you broke down this component.

Left one minor nitpick around styling and a question about whether or not we should go ahead and export this experimental component, but pre-approving as this is 💯

packages/js/components/src/experimental-tree-control/hooks/use-linked-tree.ts
function findChildren(
items: Item[],
parent?: Item[ 'parent' ],
memo: MemoItems = {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯 nicely done!

packages/js/components/src/experimental-tree-control/hooks/use-linked-tree.ts

return children.map( ( child ) => {
const linkedTree = memo[ child.value ];
linkedTree.parent = child.parent ? memo[ child.parent ] : undefined;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

packages/js/components/src/experimental-tree-control/hooks/use-tree-item.ts

export function useTreeItem( { item, level, ...props }: TreeItemProps ) {
const nextLevel = level + 1;
const nextHeadingPaddingLeft = ( level - 1 ) * 28 + 12;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we apply these styles via SCSS? This might give us more flexibility for consumers in the future.

packages/js/components/src/experimental-tree-control/index.ts
@@ -0,0 +1,4 @@
export * from './tree';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor and may not be necessary for this PR, but should we also export this under the components/src/index.ts?

@mattsherman mattsherman merged commit eca891d into trunk Jan 19, 2023
@mattsherman mattsherman deleted the add/35851-tree-control branch January 19, 2023 18:59
@github-actions github-actions bot added this to the 7.4.0 milestone Jan 19, 2023
adrianduffell pushed a commit that referenced this pull request Jan 20, 2023
@mdperez86 @adrianduffell
Create tree-control component (#36432)
031c694
@mattsherman mattsherman mentioned this pull request Jan 24, 2023
Closed
7 tasks
vedanshujain pushed a commit that referenced this pull request Jan 25, 2023
@mdperez86 @vedanshujain
Create tree-control component (#36432)
2ab39f6
@mattsherman mattsherman mentioned this pull request Jan 25, 2023
Open
17 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joshuatf joshuatf joshuatf approved these changes

Assignees

@mdperez86 mdperez86

Labels
focus: new product ux revamped product management experience focus: product management Related to product creation and editing. package: @woocommerce/components issues related to @woocommerce/components
Projects
None yet
Milestone
7.4.0
Development

Successfully merging this pull request may close these issues.

[Enhancement] Allow users to add items using keyboard
3 participants
@mdperez86 @joshuatf @mattsherman