openfoodnetwork
openfoodnetwork copied to clipboard
[BUU] Don't reload whole table when cloning products
When performing a clone from the actions menu, the whole table is reloaded. This can take some time, and is jarring because you can lose your position in a long list of products. The old products screen does this so we need to support it here too.
- After confirming, attempt to clone the product using existing behaviour (the name has "COPY" prepended to it).
- If clone succeeds, insert the row(s) directly below the cloned product.
- Try using a "slide in" animation, to avoid the jump which would cause the user to lose their place.
- If the action fails, show a flash message explaining why, without changing any other part of the page.
Other notes:
- Should the pagination info be updated ("showing x of x")? For the first iteration, make no changes, this will be re-evaluated later
- Any unsaved edits will remain on the page and not affect progress. (fixes issues when other rows have been edited as pictured)
- If the product to be cloned has unsaved changes, should they be saved and included in the clone? We'll keep it simple and won't attempt that. As usual, unsaved changes (yellow border) are unsaved until the save button is pressed.
- Depends on #12361
- Very similar to #12398
if we're viewing 15 per page, we'll see 14 after the action (if deleting), we'll see 16 if cloning.
estimate: 1day
@mariocarabotta @RachL what is the expected behaviour for the pagination info? Asking because this is generated by the backend.
Hi @anansilva, apologies we have been very light on for details here. Based on previous discussions, I've written some requirements (edit: moved to description at top), but there are still some gaps (as you have identified).
TBC:
- should the pagination info be updated?
- should there be an animation?
- If the product to be cloned has unsaved changes, should they be saved and included in the clone?
I'll share my thoughts to start with, but Mario or Rachel please confirm what you think.
- should the pagination info be updated?
I think it would be problematic to try and update it, because we're messing with the pagination. What if we make the text greyed out, to show that it's no longer accurate, but hasn't been regenerated yet?
- should there be an animation?
I would suggest a simple slide in (for clone) and slide out (for delete), to avoid the jump which would cause the user to lose their place.
- If the product to be cloned has unsaved changes, should they be saved and included in the clone?
Maybe that would be handy, but I suggest we keep it simple and don't attempt that. Unsaved changes (yellow border) are unsaved until the save button is pressed. Another option is to show a specific warning dialog ("this product has unsaved changes, do you want to submit all changes before cloning?"). But again I don't think we should attempt that. Unsaved changes are simply ignored and not cloned. They are not cleared, so can be saved after the clone is done.
And finally, as mentioned in Slack, this requires a framework which isn't ready yet, so I've marked as blocked for now.
For reference, the current bulk products screen does update the pagination count when deleting or cloning. See below example where there was a page of 15, and i deleted lots of products:
It still says "15 per page" which obviously isn't quite true in this case. But "Viewing 1 to 3" is mostly accurate. (It's not perfect though: if we're cloning product "Z" on the last page, a "COPY OF Z" is visible on screen underneath it. But when you reload the page you find that "COPY OF Z" is now on the first page.)
Because it's pushing the limits of Reactive Rails (with Turbo), I suggest we first try and solve it with design if possible. But clearly the precedent is there, and I think we could do it if needed.
Thanks for your comments @dacook, should we split this issue into smaller ones? Here's my suggestion:
- Issue/PR to fix the full page reload on delete
- Issue/PR to fix the full page reload on clone
- ... (further UI improvements on both these actions)
Thanks Ana. I've gone ahead and split into two issues: this one for cloning, and a new one for deleting. I've also updated the description to incorporate the latest discussion. If you would like to solve this with more than one PR, that is fine.
This issue is now unassigned and ready to start. Here is a work in progress that might be helpful to refer to:
- https://github.com/openfoodfoundation/openfoodnetwork/pull/12404