[BUU] Columns broken after cloning

[dacook]

Description

On the new Bulk Edit Products screen (/admin/products with admin_style_v3), And I have hidden some columns using the "Columns" dropdown,

When I select the product action Clone, A new product appears in the table, but An extra grey column appears to the side of the table (see picture)

Expected Behavior

The column sizes should remain as they were.

Actual Behaviour

as above

Steps to Reproduce

as above

Animated Gif/Screenshot

Workaround

Select all columns for display, or reload the page.

Severity

bug-s4: it's annoying, but you can use it

https://github.com/openfoodfoundation/openfoodnetwork/wiki/Bug-severity

Your Environment

• Version used:
• Browser name and version:
• Operating System and version (desktop or mobile):

Possible Fix

[dacook]

For developers:

The "new variant" button needs to span all visible columns in the table, but HTML's table doesn't have a built-in way to achieve that. So we are using JS to set the colspan whenever we change the number of columns shown with the column selector. The code adjusting the colspans is here: https://github.com/openfoodfoundation/openfoodnetwork/blob/master/app/webpacker/controllers/column_preferences_controller.js

The problem is that the newly cloned product has a "new variant" cell that spans multiple columns. Other "new variant" cells have been adjusted by ColumnPreferencesController, but this one hasn't yet:

I guess we need to tell ColumnPreferencesController to re-register the new colSpanCells. I think we might be able to add a trigger similar to BulkFormController.registerElements(). See: https://github.com/openfoodfoundation/openfoodnetwork/blob/5bb47823c64e8c4c8813212bd1506c3dc914e0fe/app/views/admin/products_v3/_product_variant_row.html.haml#L4

---

Some other thoughts:

Will it work better if we make the "new variant" cell span the entire row? We can easily give it a left padding to maintain its placement. Hmm, still need to make the colspan adjustment anyway.

It would be awesome if we could remove the need for the colspan adjustment entirely, but I don't think that will work:

• the "new variant" button could exist on the left-most column, but what if that column is hidden..
  • maybe it could exist on every column, but invisible, and only visible for the left-most column that is showing
• also what if that column isn't wide enough? we could try breaking out of the column, but last time I tried I couldn't get that to work.

[kernal053]

Hi @dacook, I would love to contribute. I am able to fix above issue using MutationObserver on product table. I tried using your above shared approach of re-register on clone action, but couldn't fetch updated colspan cells. Please let know your thoughts.

[dacook]

Hi @kernal053, great to have your enthusiasm! Thanks for trying out my suggestion. I wonder why it didn't work; would you be able to share the code changes you tried?

I'm generally cautious about introducing MutationObserver, because it feels like adding another layer on top of our app, which will potentially cause problems in the future. But I think I can see how it might fit in seamlessly, as part of column_preferences_controller.js. Is that how you imagined? Feel free to go ahead and try.

[dacook]

This is tricky, and I think it's showing that we're reaching the limits of what we can do with the "Reactive Rails" approach, using Stimulus to "sprinkle" extra functionality on top of the Rails stack. We didn't realise this at the start, but I wonder if we should have used a frontend framework like Angular for this screen..

[kernal053]

I have tried changes for re-registration on clone, please check this commit : ef67023. But couldn't fetch updated DOM even after using setTimeout.

I tried to find event that's trigger after adding turbo-stream to DOM, but couldn't find any. This conversation also address it.

So, should I go ahead with using MutationObserver ? Or we will need to use Angular for this screen ? I'm also new to Angular, will need help if we're going ahead with it.

[dacook]

Thanks for trying this out. In your commit, it looks like it's calling registerCell when you click, before the the clone has completed. I'm not sure why it won't work with setTimeout, but that wouldn't be a good solution because the timing will vary.

If there was some way to add the registerCell call to the clone response, that should work.

I have also searched for an event and found that thread! I find it disappointing that the only solution is to use MutationObserver.. I'm tempted to try hacking in a turbo:after-stream-render event as suggested. But maybe MutationObserver is the right way after all. So yes, please give that a try.

Regarding Angular, we're actually intending to remove it from the app, so don't worry about trying to use that. I was just thinking about frameworks in general..

[dacook]

PS, if you haven't seen it yet, I've written an introduction to our use of Turbo here: https://github.com/openfoodfoundation/openfoodnetwork/wiki/Turbo Sorry I didn't think to point this out earlier. I'd be happy to receive any feedback or suggestions about it too.