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.

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

Add selection to the tree control #36435

Merged
merged 16 commits into from Feb 28, 2023
Merged

Conversation

mdperez86
Copy link
Contributor

@mdperez86 mdperez86 commented Jan 14, 2023

Depends on #36590

All Submissions:

Changes proposed in this Pull Request:

Depends on #36476
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--selection-single for a single selection tree. Or http://localhost:6007/?path=/story/woocommerce-admin-experimental-treecontrol--selection-multiple for a multiple selection tree.
  3. Both examples have an initial value set.
  4. After clicking a leaf node in the single selection tree, the selected value should be change.
  5. In multiple selection tree we can select any children.
    5.1. When selecting a leaf, all its ancestors should be selected also.
    5.2 When selecting an item with children, the item and its children should be selected.
    5.3 When selecting an item with parent and children, its parent and its children should be selected.
    5.4 When unselecting a leaf, if it has selected siblings, then it becomes unselected and all its ancestors indeterminate.
    5.5 When unselecting a leaf, if it has unselected siblings, then it becomes unselected and all its ancestors unselected.
    5.6 When unselecting an item with children, all it children becomes unselected.

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 added focus: product management Related to product creation and editing. focus: new product ux revamped product management experience labels Jan 14, 2023
@mdperez86 mdperez86 requested a review from a team January 14, 2023 05:39
@mdperez86 mdperez86 self-assigned this Jan 14, 2023
@github-actions github-actions bot added the package: @woocommerce/components issues related to @woocommerce/components label Jan 14, 2023
@mdperez86 mdperez86 changed the title Add/35851 tree control selection Add selection to the tree control Jan 14, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 14, 2023

Test Results Summary

Commit SHA: 1ad730e

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202611m 4s
E2E Tests189006019515m 0s

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 marked this pull request as ready for review January 17, 2023 13:47
@mdperez86 mdperez86 force-pushed the add/35851-tree-control-selection branch from 2a0480e to 012d95e Compare January 17, 2023 15:45
@mattsherman mattsherman assigned mattsherman and unassigned mdperez86 Jan 19, 2023
@mattsherman mattsherman mentioned this pull request Jan 24, 2023
7 tasks
@mattsherman mattsherman added the status: blocked The issue is blocked from progressing, waiting for another piece of work to be done. label Jan 24, 2023
@mattsherman mattsherman marked this pull request as draft February 1, 2023 19:58
@mattsherman mattsherman removed their assignment Feb 15, 2023
@AnnaMag AnnaMag marked this pull request as ready for review February 23, 2023 14:41
@octaedro octaedro changed the base branch from trunk to add/35851-tree-control-custom-label February 24, 2023 18:56
Base automatically changed from add/35851-tree-control-custom-label to trunk February 24, 2023 20:00
@mdperez86 mdperez86 force-pushed the add/35851-tree-control-selection branch 2 times, most recently from e2268f7 to ea7f37d Compare February 24, 2023 21:28
@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #36435 (1ad730e) into trunk (be43265) will not change coverage.
The diff coverage is 27.5%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             trunk   #36435   +/-   ##
========================================
  Coverage     46.7%    46.7%           
  Complexity   17183    17183           
========================================
  Files          429      429           
  Lines        64799    64799           
========================================
  Hits         30251    30251           
  Misses       34548    34548           
Impacted Files Coverage Δ
...mmerce/includes/admin/class-wc-admin-importers.php 0.0% <0.0%> (ø)
...rters/class-wc-product-csv-importer-controller.php 36.1% <28.6%> (ø)
.../includes/import/class-wc-product-csv-importer.php 74.9% <44.4%> (ø)
...e/includes/import/abstract-wc-product-importer.php 63.7% <100.0%> (ø)

declare module '@wordpress/components' {
declare namespace CheckboxControl {
interface Props {
indeterminate?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was it necessary to add this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because for @wordpress/components@19.8.5 there is not a TS definition for the prop indeterminate. Also there is not such definition even in any version of @types/wordpress__components

Copy link
Contributor

@nathanss nathanss left a comment

Choose a reason for hiding this comment

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

It's testing well on my end! Code also looks good.

If anyone else wants to review it, I think it's a good idea, as I'm not that involved with this work.

*/
onSelect?( value: Item | Item[] ): void;
/**
* When `multipe` is true and a child item is unselected, all its
Copy link
Contributor

Choose a reason for hiding this comment

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

There's a small typo here in multipe

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! done

.experimental-woocommerce-tree-item {
margin: 0;

&--highlighted {
> .experimental-woocommerce-tree-item__heading {
background-color: #f6f7f7;
Copy link
Contributor

Choose a reason for hiding this comment

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

If we ever implement dark mode I think we would have to go through all these "hard coded" values... But it's probably out of scope of this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, it's done now. Thanks

nathanss
nathanss previously approved these changes Feb 27, 2023
@mdperez86 mdperez86 force-pushed the add/35851-tree-control-selection branch from 5795e0b to 27ff71e Compare February 27, 2023 20:43
@mdperez86 mdperez86 force-pushed the add/35851-tree-control-selection branch from 27ff71e to 17b014c Compare February 27, 2023 20:46
@mdperez86 mdperez86 force-pushed the add/35851-tree-control-selection branch from 17b014c to 1ad730e Compare February 27, 2023 21:11
@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Feb 27, 2023
@mdperez86 mdperez86 merged commit 7d0669d into trunk Feb 28, 2023
@mdperez86 mdperez86 deleted the add/35851-tree-control-selection branch February 28, 2023 18:10
@github-actions github-actions bot added this to the 7.6.0 milestone Feb 28, 2023
@mdperez86 mdperez86 self-assigned this Feb 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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 plugin: woocommerce Issues related to the WooCommerce Core plugin. status: blocked The issue is blocked from progressing, waiting for another piece of work to be done.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Allow users to add items using keyboard
3 participants