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

[BUU] Don't reload whole table when cloning products #11987

Open
mariocarabotta opened this issue Dec 28, 2023 · 8 comments
Open

[BUU] Don't reload whole table when cloning products #11987

mariocarabotta opened this issue Dec 28, 2023 · 8 comments
Assignees

Comments

@mariocarabotta
Copy link
Collaborator

mariocarabotta commented Dec 28, 2023

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.

Screenshot 2023-12-28 at 14 08 16

  1. After confirming, attempt to clone the product using existing behaviour (the name has "COPY" prepended to it).
  2. 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.
  3. 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.
@mariocarabotta mariocarabotta changed the title Don't reload whole table when individual items actions are performed Don't reload whole table when table menu actions are performed Dec 28, 2023
@mariocarabotta
Copy link
Collaborator Author

if we're viewing 15 per page, we'll see 14 after the action (if deleting), we'll see 16 if cloning.

estimate: 1day

@anansilva
Copy link
Collaborator

@mariocarabotta @RachL what is the expected behaviour for the pagination info? Asking because this is generated by the backend.

Screenshot 2024-04-15 at 19 55 05

@dacook
Copy link
Member

dacook commented Apr 16, 2024

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?

@dacook
Copy link
Member

dacook commented Apr 16, 2024

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.

@dacook dacook added the blocked label Apr 16, 2024
@dacook

This comment was marked as resolved.

@dacook
Copy link
Member

dacook commented Apr 17, 2024

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:

Screenshot 2024-04-17 at 3 30 57 pm

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.

@dacook dacook changed the title Don't reload whole table when table menu actions are performed [BUU] Don't reload whole table when table menu actions are performed Apr 17, 2024
@anansilva
Copy link
Collaborator

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)

@dacook dacook changed the title [BUU] Don't reload whole table when table menu actions are performed [BUU] Don't reload whole table when cloning products Apr 18, 2024
@dacook
Copy link
Member

dacook commented Apr 18, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress ⚙
Development

No branches or pull requests

4 participants