LPD-24920 Implements the new ClayTable component to FDS #602
Conversation
|
To conserve resources, the PR Tester does not automatically run for every pull. If your code changes were already tested in another pull, reference that pull in this pull so the test results can be analyzed. If your pull was never tested, comment "ci:test" to run the PR Tester for this pull. |
matuzalemsteles
changed the title
|
Well, I sent new changes now working with the column resizer with the FDS implementation using the new Clay Table. There are still some more tests to be done in use cases:
I'm also putting together a new test plan, some of my ideas are to introduce unit tests to test interactions and ensure the integration of the FDS operation with the new ClayTable.
This is almost complete and this should close the tickets: |
matuzalemsteles
force-pushed
the
LPD-24920
branch
from
4a5224f to
b0ec5b0
Compare
|
@matuzalemsteles let me know when this PR is ready to be reviewed. |
|
@ethib137 I believe this may already have a review round, I'm starting to work on the tests now. The use case for inline edit Markos commented to me that this case is old and doesn't work and that they are still going to work on it, so I'm going to discard this case here and the nested row we are seeing. |
|
Just an update on this, I'm working on testing with playwright but I'm still having some problems due to some inconsistencies in the tests and it's not creating the layout 😬. So maybe I'll just send unit tests to move this forward and we can work on those end to end tests later. |
There was a problem hiding this comment.
Looks good. This provides a much nicer experience on sorting. The page doesn't flash like it did before. One thing to check, is that all the columns can be hidden. I'm not sure if that's a bug here or a bug in Clay.
Nice Work @matuzalemsteles !
...ta-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/table/ClayTable.tsx
Show resolved
Hide resolved
| column | ||
| ); | ||
|
|
||
| let cmp = new Intl.Collator('en', {numeric: true}).compare( |
There was a problem hiding this comment.
Better to not abbreviate on the variable name.
There was a problem hiding this comment.
Also should 'en' be a locale variable that's passed in?
There was a problem hiding this comment.
Also should 'en' be a locale variable that's passed in?
Hm yes, it's good when the data is written in another language, I'll check if this is really necessary and do some tests.
...ta-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/table/ClayTable.tsx
Show resolved
Hide resolved
...ta-set/frontend-data-set-web/src/main/resources/META-INF/resources/views/table/ClayTable.tsx
Show resolved
Hide resolved
From what I understand in the case of FDS, only the select column and the actions column are always visible but the rest are not, have you seen anything that goes beyond this? |
I tested this in Commerce products index page and in that case it won't let you remove all the labeled columns. It still requires at least one to be shown. Additionally when we added this feature to the clay table we decided this would be a requirement. This is why we added the text in the column visibility dropdown that says: "At least one column must remain visible." So let's ensure the behavior works this way. |
|
Looks like this also needs to be rebased. |
Actually I didn't quite understand what you meant, so should we also add the selectable to be hidden? I also noticed that there is a bug that when the selectable is always visible, Clay is letting all the columns be hidden, I'll fix that. |
matuzalemsteles
force-pushed
the
LPD-24920
branch
from
b0ec5b0 to
db825a3
Compare
|
ci:test:sf |
liferay-continuous-integration
added
ci:test:sf - pending
ci:test:sf - failure
and removed
ci:test:sf - pending
labels
❌ ci:test:sf - 0 out of 1 jobs passed in 4 minutesClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPD-24920 1 Failed Jobs:For more details click here. [exec] yarn install v1.13.0
[exec] warning Missing name in workspace at "/opt/dev/projects/github/liferay-portal/modules/dxp/apps/osb/osb-faro", ignoring.
[exec] [1/4] Resolving packages...
[exec] warning Resolution field "@types/html-minifier-terser@5.1.0" is incompatible with requested version "@types/html-minifier-terser@^6.0.0"
[exec] warning Resolution field "@liferay/eslint-plugin@1.5.0" is incompatible with requested version "@liferay/eslint-plugin@1.4.0"
[exec] warning Resolution field "webpack@5.89.0" is incompatible with requested version "webpack@5.76.0"
[exec] success Already up-to-date.
[exec] Done in 1.25s.
[exec]
[exec] > Task :packageRunCheckFormat
[exec] yarn run v1.13.0
[exec] \$ node-scripts format --check
[exec] Error: Format checked 12 files
[exec] 2 files have problems
[exec]
[exec] at format (file:///opt/dev/projects/github/liferay-portal/modules/_node-scripts/format/index.mjs:276:9)
[exec] error Command failed with exit code 1.
[exec] info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[exec]
[exec] > Task :packageRunCheckFormat FAILED
[exec] Gradle build finished at 2024-06-11 03:51:53.180.
[exec] 3 actionable tasks: 2 executed, 1 up-to-date
[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] > 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) |
|
Jenkins Build:test-portal-source-format#7787 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-platform-experience#602 Testray Routine:EE Pull Request Testray Build:[master] ci:test:sf - matuzalemsteles > liferay-platform-experience - PR#602 - 2024-06-10[20:47:39] Testray Build ID:14360338 Testray Importer:publish-testray-report#4154 |
|
ci:test:sf |
❌ ci:test:sf - 0 out of 1 jobs passed in 4 minutesClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPD-24920 1 Failed Jobs:For more details click here. [exec]
[exec] > Task :yarnInstall
[exec] Using registry http://mirrors.lax.liferay.com:4873
[exec] yarn install v1.13.0
[exec] warning Missing name in workspace at "/opt/dev/projects/github/liferay-portal/modules/dxp/apps/osb/osb-faro", ignoring.
[exec] [1/4] Resolving packages...
[exec] warning Resolution field "@types/html-minifier-terser@5.1.0" is incompatible with requested version "@types/html-minifier-terser@^6.0.0"
[exec] warning Resolution field "@liferay/eslint-plugin@1.5.0" is incompatible with requested version "@liferay/eslint-plugin@1.4.0"
[exec] warning Resolution field "webpack@5.89.0" is incompatible with requested version "webpack@5.76.0"
[exec] success Already up-to-date.
[exec] Done in 1.36s.
[exec]
[exec] > Task :packageRunCheckFormat
[exec] yarn run v1.13.0
[exec] \$ node-scripts format --check
[exec] Error: Format checked 12 files
[exec] 2 files have problems
[exec]
[exec] at format (file:///opt/dev/projects/github/liferay-portal/modules/_node-scripts/format/index.mjs:276:9)
[exec] error Command failed with exit code 1.
[exec] info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
[exec]
[exec] > Task :packageRunCheckFormat FAILED
[exec] Gradle build finished at 2024-06-11 06:50:22.922.
[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] > 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) |
|
Jenkins Build:test-portal-source-format#6908 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-platform-experience#602 Testray Routine:EE Pull Request Testray Build:[master] ci:test:sf - matuzalemsteles > liferay-platform-experience - PR#602 - 2024-06-10[23:46:53] Testray Build ID:14463666 Testray Importer:publish-testray-report#26402 |
|
Oh I see, ok. Makes sense now. I have already sent the fix to Clay and included it in the release. |
|
ci:test:sf |
✔️ ci:test:sf - 1 out of 1 jobs passed in 5 minutesClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPD-24920 1 Successful Jobs:For more details click here. |
|
Jenkins Build:test-portal-source-format#9635 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-platform-experience#602 Testray Routine:EE Pull Request Testray Build:[master] ci:test:sf - matuzalemsteles > liferay-platform-experience - PR#602 - 2024-06-25[12:18:07] Testray Build ID:25881534 Testray Importer:publish-testray-report#37090 |
✔️ ci:test:stable - 32 out of 32 jobs passed❌ ci:test:relevant - 39 out of 42 jobs passed in 2 hours 12 minutesClick here for more details.This pull is eligible for reevaluation. When this upstream build has completed, using the following CI command will compare this pull request result against a more recent upstream result: ci:reevaluate:1337401_6276 Base Branch:Branch Name: master Upstream Comparison:Branch GIT ID: abc4222371a1a253ae05ecb3e6083858ac965415 ci:test:stable - 32 out of 32 jobs PASSED32 Successful Jobs:ci:test:relevant - 39 out of 42 jobs PASSED3 Failed Jobs:
39 Successful Jobs:For more details click here.Failures unique to this pull:Test bundle downloads:
|
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#6276 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-platform-experience#602 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - matuzalemsteles > liferay-platform-experience - PR#602 - 2024-06-25[12:18:03] Testray Build ID:26069205 Testray Importer:publish-testray-report#13689 |
|
ci:forward |
|
CI is automatically triggering the following test suites:
The pull request will automatically be forwarded to the user
|
|
Skipping previously passed test suites:
|
✔️ ci:test:stable - 32 out of 32 jobs passed✔️ ci:test:relevant - 41 out of 42 jobs passed in 1 hour 32 minutesClick here for more details.Base Branch:Branch Name: master Upstream Comparison:Branch GIT ID: abc4222371a1a253ae05ecb3e6083858ac965415 ci:test:stable - 32 out of 32 jobs PASSED32 Successful Jobs:ci:test:relevant - 40 out of 42 jobs PASSED2 Failed Jobs:
40 Successful Jobs:For more details click here.This pull contains no unique failures.Test bundle downloads:
|
|
All required test suite(s) passed. |
|
Pull request has been successfully forwarded to brianchandotcom#151748 |
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#10128 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-platform-experience#602 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - matuzalemsteles > liferay-platform-experience - PR#602 - 2024-06-25[14:57:04] Testray Build ID:26175356 Testray Importer:publish-testray-report#21431 |
The new ClayTable component now handles more responsibilities and has a new improved and less verbose DX removing the excess drilling prop and verbosity of the old component as well as OOTB features such as nested row, sorting columns and visibility columns the component is accessibility first.
This is under feature flag and still WIP, in this initial implementation we are not adding the column resizer feature we will use the same FDS implementation while we did not work on it in Clay.
We also need to do some more nested row tests and add a new row with inline edit. We need to wait for the Clay #600 release PR to come in so we can do more tests with the fixes I sent to Clay.
After that I will add some before and after screenshots showing the features.