-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 ability to select multiple rows on Admin Tables #3565
Add ability to select multiple rows on Admin Tables #3565
Conversation
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table.js
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table.js
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table.js
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table.js
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table.js
Outdated
Show resolved
Hide resolved
1eae7eb
to
3e3a792
Compare
3e3a792
to
e3eaaba
Compare
e3eaaba
to
d79be59
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple of notes, only one is blocking. Great work Daniele!
backend/app/assets/javascripts/spree/backend/components/selectable_table.js
Outdated
Show resolved
Hide resolved
backend/app/views/spree/admin/style_guide/topics/tables/_building_tables.html.erb
Outdated
Show resolved
Hide resolved
backend/app/views/spree/admin/style_guide/topics/tables/_building_tables.html.erb
Outdated
Show resolved
Hide resolved
da9f15f
to
0314317
Compare
backend/app/views/spree/admin/style_guide/topics/tables/_building_tables.html.erb
Outdated
Show resolved
Hide resolved
5eb017f
to
f2f9054
Compare
We discussed IRL, and we are trying to move more things to handlebars templates. |
f2f9054
to
5e3dcab
Compare
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table_sum_item_amount.js
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table_sum_item_amount.js
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table_sum_item_amount.js
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table.js
Outdated
Show resolved
Hide resolved
5e3dcab
to
e8382d0
Compare
Refactored with handlebars templates |
e8382d0
to
b9ce553
Compare
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table_sum_item_amount.js
Outdated
Show resolved
Hide resolved
cb87120
to
22abbc7
Compare
22abbc7
to
6a507d1
Compare
@DanielePalombo Not sure how, but the failure doesn't seem a flaky, can you take a look? I'm restarting the suite again just be sure anyway. |
@kennyadsl It looks green now |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this also persisting any of the changes to the server? Is this only for removing columns or also for exerting multiple items at once?
|
||
new Spree.Views.Tables.SelectableTable({el: this, model: selectableTableModel}); | ||
|
||
if($(this).hasClass('return-items-table')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why hard-code this behavior here? Can we add a dedicated view model for the return items table instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a dedicated view for return items table, let me know if it works for you.
backend/app/assets/javascripts/spree/backend/templates/tables/selectable_label.hbs
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/templates/tables/selectable_label.hbs
Outdated
Show resolved
Hide resolved
|
||
img { | ||
max-width: none; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this change necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
backend/app/assets/javascripts/spree/backend/components/selectable_table.js
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table_summary.js
Outdated
Show resolved
Hide resolved
backend/app/assets/javascripts/spree/backend/views/tables/selectable_table_summary.js
Outdated
Show resolved
Hide resolved
b5d14b7
to
cdee859
Compare
9fb2dfb
to
83869e7
Compare
This commit adds the ability to select more rows on admin resource tables. To use this feature, add the `selectable-table` CSS class to the selected table and the CSS class `selectable` to the checkbox, which identifies the record id. It is possible to add a checkbox named `select-all` to check/uncheck all the elements. An example of its usage is provided at `/admin/style_guide` under `Building Tables` section.
This event is triggered when the selectableTable is initialized. Binding on this event you can add your own logic to the selectableTable. e.g. ``` # app/assets/javascript/components/selectable_table/custom.js Backbone.on('selectableTable:init', function(selectableTable){ if(selectableTable.$el.find('.selectable').length > 0) { new Spree.Views.Tables.SelectableTable.Custom(el: selectableTable.el); } }) ```
2b6a462
to
399c6bf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DanielePalombo thank you! 💯
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a non-blocking comment if you have time to fix it. Thanks!
backend/app/assets/javascripts/spree/backend/views/tables/return_items.js
Outdated
Show resolved
Hide resolved
399c6bf
to
fa9330d
Compare
@kennyadsl the build is failed because of flaky spec, do you mind re-running the CI? |
This component will be rendered above the selectableTable and will shown the selected item number label.
This commit updates the JS and HTML code related to return items, to use the selectable.js code. You can see the code working on: /admin/orders/ORDER_NUMBER/customer_returns/new /admin/orders/ORDER_NUMBER/return_authorizations/new return_item_selection.js file is removed in favor of HBS template.
fa9330d
to
ea23be2
Compare
This feature got lost with solidusio#3565. It automatically checks rows where the admin changes something, under the assumption that, if a value is changed, then the user means to include the row into the RMA/return. An integration spec is added to properly track this feature.
This feature got lost with solidusio#3565. It automatically checks rows where the admin changes something, under the assumption that, if a value is changed, then the user means to include the row into the RMA/return. An integration spec is added to properly track this feature.
This feature got lost with solidusio#3565. It automatically checks rows where the admin changes something, under the assumption that, if a value is changed, then the user means to include the row into the RMA/return. An integration spec is added to properly track this feature.
This feature got lost with #3565. It automatically checks rows where the admin changes something, under the assumption that, if a value is changed, then the user means to include the row into the RMA/return. An integration spec is added to properly track this feature.
Description
This PR adds the ability to select more elements of admin tables.
Before this PR the ability was present only for return items and with a custom js code.
Adding a
selectable-table
CSS class to an admin table, and a checkbox (with aselectable
class) for every related record, it will be possible to select more than one element.If a checkbox named
select-all
is present this check/uncheck all the elements.Adding the selector CSS class
selectable-label
on a node HTML it willbe rendered a text depending on the number of the selected row.
Thanks to it will be possible to show different information depending on the selection type.
e.g.
An example of usage is provided at
/admin/style_guide
underBuilding Tables
section.This PR uses the selectable code on the return items
Checklist: