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

LPD-42578 Add "Clear" Action to Remove Selection #4763

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

chestofwonders
Copy link
Collaborator

Issue: https://liferay.atlassian.net/browse/LPD-42578

Add "Clear" Action to Remove Selection

⚠️ FF LPD-42570 must be enabled

The goal of this PR is to provide a button to clear all selected items in data sets

image

Acceptance Criteria

  • When some items on any page are selected, it's shown a "Clear" button
  • Clicking the "Clear" button deselect all items, including those across other pages
  • The button works for all visualization modes

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 5 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 26ff042a7230aca438731909c0ff739a4ecaa938

Sender Branch:

Branch Name: LPD-42578
Branch GIT ID: e1a799101f6affe07f9cfd6b887ebc4a9d15c293

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@liferay-continuous-integration
Copy link
Collaborator

href="#"
onClick={(event) => {
event.preventDefault();
deselectItems(selectedItemsValue);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just wondering if we can pass a null value or a string (all) to this function so we can set the state as and empty array instead of iterating over all items in the FrontendDataSet#deselectItems?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm personally more on the side of keeping the API of the methods simple... how about asking a question at the beginning of the implementation of deselectItems? Asking if selectedItemsValues ​​has the same length as the parameter sent, should work... 😔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree on having a simple API. What you suggest is fine 👌

Copy link
Collaborator

@juanjofgliferay juanjofgliferay left a comment

Choose a reason for hiding this comment

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

LGTM. Just a question inline

Copy link
Collaborator

@markocikos markocikos left a comment

Choose a reason for hiding this comment

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

@chestofwonders please see comment inline

@@ -43,6 +43,7 @@ function ManagementBar({
{selectionType === 'multiple' && (
<BulkActions
bulkActions={bulkActions}
deselectItems={deselectItems}
Copy link
Collaborator

@markocikos markocikos Feb 28, 2025

Choose a reason for hiding this comment

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

While this is technically correct, it's a bad pattern. You never want to push callbacks down to child components. The reason is it violates separation of concerns; with this your child component contains logic that is implemented on the parent level.

Instead, you want to have a generic callback in the child component, that makes sense for that component in isolation. Something like onClear or onClearClick or onClearButtonClick prop in BulkActions, that is propagated through onBulkActionsClear prop in ManagementBar, and ultimately implemented where the state is, in FrontendDataSet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've refactored it. Probably there's more stuff around the whole FrontendDataSet structure we want to address... maybe make an more extensive use of the Context, but for now should be enough, I think

Copy link
Collaborator

@markocikos markocikos left a comment

Choose a reason for hiding this comment

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

@chestofwonders looks good, just a small nit in comment

Copy link
Collaborator

@markocikos markocikos left a comment

Choose a reason for hiding this comment

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

@chestofwonders LGTM! ✨

@chestofwonders
Copy link
Collaborator Author

ci:forward

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:relevant
  •     ci:test:sf
  •     ci:test:stable

The pull request will automatically be forwarded to the user brianchandotcom If the following test suites pass:

  •     ci:test:relevant
  •     ci:test:sf
  •     ci:test:stable

@liferay-continuous-integration
Copy link
Collaborator

❌ ci:test:relevant - 0 out of 1 jobs passed in 7 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 4281f9b41f4c84911eaaf42439d4bf4905772f58

Upstream Comparison:

Branch GIT ID: 4281f9b41f4c84911eaaf42439d4bf4905772f58
Jenkins Build URL: EE Development Acceptance (master) - 1211 - 2025-03-03[11:50:25]

ci:test:relevant - 0 out of 1 jobs PASSED
For more details click here.

Failures unique to this pull:

  1. test-portal-acceptance-pullrequest(master)
    Job Results:

    0 Jobs Passed.
    1 Job Failed.

    A CI failure has occurred. CI Infrastructure has been notified. Please try again later or contact CI Infrastructure for an update.

For upstream results, click here.

Test bundle downloads:

@liferay-continuous-integration
Copy link
Collaborator

❌ ci:test:sf - 0 out of 1 jobs passed in 10 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 4281f9b41f4c84911eaaf42439d4bf4905772f58

Sender Branch:

Branch Name: LPD-42578
Branch GIT ID: eda8aedf32aa91bba666af10c5368074b644264f

0 out of 1jobs PASSED
For more details click here.
     [exec] > Task :packageRunCheckFormat
     [exec] yarn run v1.13.0
     [exec] \$ node-scripts check:ci
     [exec] 
     [exec] ⚙️ Running preflight checks...
     [exec] 
     [exec] ⚙️ Checking outdated tsconfig.json files ...
     [exec] 
     [exec] ⚙️ Running TypeScript checks on modified files...
     [exec] ℹ️ A total of 12 CPUs were detected: launching tsc using 12 workers
     [exec] 
     [exec] ⚙️ Running format checks on modified files...
     [exec]    /opt/dev/projects/github/liferay-portal/modules/apps/frontend-data-set/frontend-data-set-web/src/main/resources/META-INF/resources/management_bar/ManagementBar.js
     [exec]      35:11  error  'deselectItems' is not defined.  no-undef
     [exec]    
     [exec]    ✖ 1 problem (1 error, 0 warnings)
     [exec]    
     [exec]    
     [exec] ❌ CI checks failed.
     [exec] info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
     [exec] error Command failed with exit code 1.
     [exec] 
     [exec] > Task :packageRunCheckFormat FAILED
     [exec] 
     [exec] 
     [exec] FAILURE: Build failed with an exception.
     [exec] 
     [exec] * What went wrong:
     [exec] Execution failed for task ':packageRunCheckFormat'.
     [exec] > Process 'command '/opt/dev/projects/github/liferay-portal/build/node/bin/node'' finished with non-zero exit value 1
     [exec] 
     [exec] * Try:
     [exec] > Run with --info or --debug option to get more log output.
     [exec] > Run with --scan to get full insights.
     [exec] > Get more help at https://help.gradle.org.
     [exec] 
     [exec] * Exception is:
     [exec] org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':packageRunCheckFormat'.
     [exec]   at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.lambda\$executeIfValid\$1(ExecuteActionsTaskExecuter.java:148)
     [exec]   at org.gradle.internal.Try\$Failure.ifSuccessfulOrElse(Try.java:282)
     [exec]   at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeIfValid(ExecuteActionsTaskExecuter.java:146)
     [exec]   at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:134)
     [exec]   at org.gradle.api.internal.tasks.execution.FinalizePropertiesTaskExecuter.execute(FinalizePropertiesTaskExecuter.java:46)
     [exec]   at org.gradle.api.internal.tasks.execution.ResolveTaskExecutionModeExecuter.execute(ResolveTaskExecutionModeExecuter.java:51)

@liferay-continuous-integration
Copy link
Collaborator

@liferay-continuous-integration
Copy link
Collaborator

@chestofwonders
Copy link
Collaborator Author

ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 10 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 4281f9b41f4c84911eaaf42439d4bf4905772f58

Sender Branch:

Branch Name: LPD-42578
Branch GIT ID: cc10d62588c40340f6babcbc7a219b572b911bfe

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@chestofwonders
Copy link
Collaborator Author

ci:forward

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:relevant
  •     ci:test:sf
  •     ci:test:stable

The pull request will automatically be forwarded to the user brianchandotcom If the following test suites pass:

  •     ci:test:relevant
  •     ci:test:sf
  •     ci:test:stable

@liferay-continuous-integration
Copy link
Collaborator

Skipping previously passed test suites:

  • ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

❌ ci:test:relevant - 0 out of 1 jobs passed in 7 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 4281f9b41f4c84911eaaf42439d4bf4905772f58

Upstream Comparison:

Branch GIT ID: 4281f9b41f4c84911eaaf42439d4bf4905772f58
Jenkins Build URL: EE Development Acceptance (master) - 1211 - 2025-03-03[11:50:25]

ci:test:relevant - 0 out of 1 jobs PASSED
For more details click here.

Failures unique to this pull:

  1. test-portal-acceptance-pullrequest(master)
    Job Results:

    0 Jobs Passed.
    1 Job Failed.

    A CI failure has occurred. CI Infrastructure has been notified. Please try again later or contact CI Infrastructure for an update.

For upstream results, click here.

Test bundle downloads:

@liferay-continuous-integration
Copy link
Collaborator

@liferay-continuous-integration
Copy link
Collaborator

@chestofwonders
Copy link
Collaborator Author

ci:test:relevant

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:stable - 24 out of 24 jobs passed

❌ ci:test:relevant - 26 out of 30 jobs passed in 1 hour 48 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 4281f9b41f4c84911eaaf42439d4bf4905772f58

Upstream Comparison:

Branch GIT ID: 4281f9b41f4c84911eaaf42439d4bf4905772f58
Jenkins Build URL: EE Development Acceptance (master) - 1211 - 2025-03-03[11:50:25]

ci:test:stable - 24 out of 24 jobs PASSED
24 Successful Jobs:
    ci:test:relevant - 26 out of 30 jobs PASSED

    4 Failed Jobs:

    26 Successful Jobs:
      For more details click here.

      Failures unique to this pull:

      1. playwright-js-tomcat90-mysql57/2/0 - Playwright Report
        1. frontend-data-set-admin-web/tests/data-set-admin/creationActions.spec.ts
        2. frontend-data-set-admin-web/tests/data-set-admin/clientExtensionFilters.spec.ts
        3. frontend-data-set-admin-web/tests/data-set-admin/dataSetsPermissions.spec.ts
        4. ...
      2. playwright-js-tomcat90-mysql57/2/1 - Playwright Report
        1. frontend-data-set-admin-web/tests/data-set-fragment/itemActions.spec.ts
        2. frontend-data-set-admin-web/tests/data-set-admin/visualizationModes.spec.ts
        3. frontend-data-set-admin-web/tests/data-set-fragment/dataSets.spec.ts
        4. ...

      Failures in common with acceptance upstream results at 4281f9b:
      Test bundle downloads:

      @liferay-continuous-integration
      Copy link
      Collaborator

      Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
      Projects
      None yet
      Development

      Successfully merging this pull request may close these issues.

      4 participants