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 dropdown with actions buttons for selected items inside DataTable #902

Merged
merged 4 commits into from
Oct 7, 2024

Conversation

BenjaminCharmes
Copy link
Contributor

@BenjaminCharmes BenjaminCharmes commented Oct 2, 2024

Closes #897

This PR refactors the "Add to Collection" and "Delete Selected" actions into a PrimeVue SplitButton dropdown.
Also add a small fix to make cypress e2e tests work with dropdown.

Copy link

codecov bot commented Oct 2, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.34%. Comparing base (65a2660) to head (e4d1745).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #902   +/-   ##
=======================================
  Coverage   68.34%   68.34%           
=======================================
  Files          62       62           
  Lines        3921     3921           
=======================================
  Hits         2680     2680           
  Misses       1241     1241           

Copy link

cypress bot commented Oct 2, 2024

datalab    Run #2513

Run Properties:  status check passed Passed #2513  •  git commit 6dde13dff8 ℹ️: Merge e4d17459c946a423a5454a024ffb3c7301c775b6 into 65a2660c66e8593b85db8997ac3e...
Project datalab
Run status status check passed Passed #2513
Run duration 06m 47s
Commit git commit 6dde13dff8 ℹ️: Merge e4d17459c946a423a5454a024ffb3c7301c775b6 into 65a2660c66e8593b85db8997ac3e...
Committer Benjamin Charmes
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 396

@BenjaminCharmes
Copy link
Contributor Author

Hey @jdbocarsly,
Here's a first draft to see what it might look like. Don't hesitate to tell me if you want me to modify the css style, or the text ...

Screenshot 2024-10-02 at 12 01 48

@BenjaminCharmes BenjaminCharmes force-pushed the bc/dynamic-button-dropdown branch 2 times, most recently from 1cc6919 to 3eeb595 Compare October 4, 2024 11:05
Copy link
Member

@jdbocarsly jdbocarsly left a comment

Choose a reason for hiding this comment

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

Styling looks great! just a few minor changes

webapp/src/components/DynamicButtonDataTable.vue Outdated Show resolved Hide resolved
webapp/src/components/DynamicButtonDataTable.vue Outdated Show resolved Hide resolved
Add dropdown actions buttons for selected items inside DataTable

Fix e2e cypress test to work with dropdown

Fix e2e cypress test to work with dropdown

Add dropdown actions buttons for selected items inside DataTable
Fix e2e cypress test to work with dropdown
Add a few minor changes
ml-evs
ml-evs previously approved these changes Oct 7, 2024
Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

Looks great to me, thanks @BenjaminCharmes! It looks like you've incorporated all Josh's changes so I'm happy to merge once after adding one tiny change

@@ -318,6 +319,7 @@ describe.only("Advanced sample creation features", () => {
// Visit collections page and look for 'test_collection'
cy.visit("/collections");
// Visit edit page of collection and check that the sample is there
cy.get('[data-testid="search-input"]').type("test_collection");
Copy link
Member

Choose a reason for hiding this comment

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

Ah I now see what you meant the other day, this is a nice solution!

webapp/src/components/DynamicDataTableButtons.vue Outdated Show resolved Hide resolved
@ml-evs ml-evs added the webapp For issues/PRs pertaining to the web interface label Oct 7, 2024
@ml-evs ml-evs merged commit 1bc6beb into main Oct 7, 2024
20 checks passed
@ml-evs ml-evs deleted the bc/dynamic-button-dropdown branch October 7, 2024 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
webapp For issues/PRs pertaining to the web interface
Projects
None yet
3 participants