openfoodnetwork icon indicating copy to clipboard operation
openfoodnetwork copied to clipboard

[BUU] Change the columns to be shown in my catalogue

Open mariocarabotta opened this issue 1 year ago • 15 comments

Context

This issue introduces the ability to change which columns to display in the table, and saving it as the default.

Description

- As an: enterprise user - On page: /admin/products - I want to be able to: change the columns in my catalogue - So that: I can see the information I need to manage my products effinciently

Acceptance Criteria & Tests

~~Scenario 1: Default - multiple producers~~ MOVED TO #11200

~~Scenario 2: Default - single producer~~ MOVED TO #11200

Scenario 3: Open dropdown Given that my catalogue has loaded successfully When I click on the columns button Then I see a dropdown with all the available columns And the ones displayed are selected

Scenario 4: Deselect column Given the columns dropdown is open When I de-select an option Then the column gets hidden from the table

Scenario 5: Select column Given the columns dropdown is open When I select an option Then the column gets displayed in the table

Scenario 6: Close dropdown without saving Given the columns dropdown is open And I have changed the columns to be displayed When I close it Then I see the selected columns only for the current session

Scenario 7: Save default Given the columns dropdown is open And I have changed the columns to be displayed When I save them as default Then the systems requests to save them And I see a loading indicator

Scenario 8: Save successful Given I am saving the default columns to display When saving is successful Then I see a success message And the dropdown closes after 2 seconds And the next time I load the page I will see the saved default columns

Scenario 9: Save failed Given I am saving the default columns to display When saving is not successful Then I see a fail message

Design here > https://www.figma.com/file/ddL6h7H9It5ZmUuVYQxzAD/Product-List?type=design&node-id=544%3A11446&mode=design&t=r4sHF7R9Mxy8Lpx2-1

Design specs

Figma screens are available here > https://www.figma.com/file/v1zbrWDZSRd3Nqoe0SJ2Sm/Engineering-Delivery---Back-Office?type=design&node-id=489%3A5377&mode=design&t=IIxsDCFfXpzBuHIb-1

Prototype here (open dropdown, select on hand, save) - note: this won't update the table underneath, it's more to show the dropdown itself > https://www.figma.com/proto/v1zbrWDZSRd3Nqoe0SJ2Sm/Engineering-Delivery---Back-Office?page-id=489%3A5377&type=design&node-id=489-5835&viewport=598%2C478%2C0.26&t=tqJxBkJp8YYHxusL-1&scaling=min-zoom&mode=design

New components and styles

Component Example
Dropdown with multiselect Screenshot 2023-08-01 at 10 14 50
Inline saving Screenshot 2023-08-01 at 10 15 08
Inline error Screenshot 2023-08-01 at 10 15 13

mariocarabotta avatar Jun 19 '23 00:06 mariocarabotta

I guess this is ready for dev since it seems to have been designed already. Or do you want me to create an interactive prototype that shows all the scenarios, so it's easier to visualize and understand?

NickIkenma avatar Sep 30 '23 02:09 NickIkenma

a similar inline button loading element has been implemented in the Connected Apps section in settings for Australia

https://staging.openfoodnetwork.org.au/admin/enterprises/anglesea-riverbank-market/edit#/connected_apps_panel

Let's talk about it during kickoff to avoid rebuilding too much

Screenshot 2024-01-04 at 14 18 15

mariocarabotta avatar Jan 04 '24 05:01 mariocarabotta

scenario 1 and 2 not considered for this estimate, will be developed separately (note in https://github.com/openfoodfoundation/openfoodnetwork/issues/11200)

mariocarabotta avatar Feb 20 '24 00:02 mariocarabotta

Estimate

  1. Add column menu viewcomponent. Consider copying markup from columns_dropdown.html.haml as a starting point. Consider subclass of modal. But probably not.
  2. Add JS controller to component, to show/hide cols
  3. ~~Add SR to~~ save preferences. with error handling.

~~1day~~

edit: Before starting, it will be worth a dev discussion. To consider:

  • Is it fine to just hide columns on the frontend? (I think it is, that means they're always there and can be instantly re-enabled).
    • Not sure of the best method to hide a table column, but one way would be to add a class to each element in the column (col/th/td), eg col_producer.
  • Will we need to tweak column widths depending on selection? (I'm hoping not, and that the table will just naturally flow well, based on the percentages already configured).
  • How much of the existing Angular JS (incl tests) can be adapted to save time on the new JS controller?
  • Method of saving preferences (AJAX)
    • should we utilise Turbo? It might be possible to create a standard Rails form for submitting to the existing ColumnPreferencesController#bulk_update action. A html or turbo_stream response format can provide the result message, while maintaining backwards compatibility for json.

dacook avatar Feb 20 '24 01:02 dacook

@mariocarabotta to update, remove button and have autosave. we'll create a separate card for adding a button if people ask for it

mariocarabotta avatar Feb 20 '24 04:02 mariocarabotta

Just pointing out that we are currently considering an alternative to StimulusReflex, so the second part of this (scenarios 7,8,9) will need further discussion.

dacook avatar Apr 08 '24 10:04 dacook

@mariocarabotta to update, remove button and have autosave.

From memory, this is preferred because it's easier for the user, it's one less click. And it doesn't matter if they forget to save. It could perhaps be annoying if they didn't want to save it, but it's a small list so quite easy to change back as needed. It's probably a good time to do it while implementing the new design, people might be happier to accept the new behaviour.

But I think we might need a design for autosave. I can envisage two ways to design it:

  1. Save on each click, and blur and show status as per current design. It should be quick to save, but if it's not it would be quite annoying if you wanted to click multiple checkboxes, having to wait for it to save between each one.
  • 1.1 perhaps we could avoid the blurring part, and allow the user to keep clicking. (this could fire off several requests in quick succession, but is probably fine)
  • It's probably fine either way, as long as they're implemented efficiently (with turbo streams or json layouts).
  1. Save when closing the popup (click or tab outside). But then we have to show status some other way

Alternatively we stick to current behaviour as designed and described above. It's hard to say if any of these options would be quicker to build or not, but I suspect the current design would be quickest.

So, I think we can do option 1 without needing further design. I would implement it with a small turbo frame for the purpose of showing the status (saving/saved/failed), that way we don't need any custom JS.

dacook avatar May 06 '24 04:05 dacook

I would not include any blurring, and I would consider starting without any saving feedback at all. Saving calls can succeed or fail in the background. If we find this a bit too jarring / incomplete, we can add some loading / success / failure later.

What do you think?

mariocarabotta avatar May 06 '24 07:05 mariocarabotta

@mariocarabotta @dacook I'm sorry, I feel this comment comes after the party (I missed the previous comment about autosave), but are we sure we want to introduce autosave as part of the first release? I understand it may saves us time when developing, but we need to consider testing and potential bugs during first release. Given we are a bit late with BUU first launch, I'm a bit anxious to add something new now :/ But happy to discuss it.

RachL avatar May 06 '24 11:05 RachL

@mariocarabotta @dacook I'm sorry, I feel this comment comes after the party (I missed the previous comment about autosave), but are we sure we want to introduce autosave as part of the first release? I understand it may saves us time when developing, but we need to consider testing and potential bugs during first release. Given we are a bit late with BUU first launch, I'm a bit anxious to add something new now :/ But happy to discuss it.

that's fair! I am not particularly attached to autosave, I think we were discussing making it a bit leaner (from an interaction point of view) a while ago, but the context and timeframe was different at that time.

Given that having a manual save seems like it would be the easiest solution from a technical point of view, I am happy if we go ahead with that.

mariocarabotta avatar May 07 '24 00:05 mariocarabotta

Hi @dacook , @mariocarabotta - I found we are using multi-select dropdown in the reports as well and here's its design: image

However, as per the given mockup we want this design: image

Just wanted to confirm the path forward before moving forward:

  1. Should we create the new multi-select dropdown as per the product's design?
  2. Should reuse the existing component?
  3. Should we update the existing report's dropdown and reuse it here?

Thanks.

chahmedejaz avatar May 16 '24 07:05 chahmedejaz

I believe it would be good to implement the new design. the ones in report have uppercase (hard to read), some spacing issues, no colors. But also conscious of timeframes, @RachL it would be good to get your view as well. We can also go ahead with using the existing report dropdown to speed things up.

mariocarabotta avatar May 20 '24 01:05 mariocarabotta

Thanks all. I suggest that we investigate if it's very easy to reuse the existing (Angular) component on this page. If it is easy, let's do that for the first iteration.

But if it's not easy, we should go ahead with building a new component. (I think we can still re-use the /admin/column_preferences/bulk_update endpoint though, see usage inapp/assets/javascripts/admin/index_utils/services/columns.js.coffee).

And so I don't think it's worth updating the report's dropdown yet. We can make the decision about that after this is done first.

dacook avatar May 20 '24 02:05 dacook

Yes unfortunately we need to aim at the quickest solutions now for BUU. Not only in terms of timeframes, but also in terms of budget (yes I know that rebuilding means more budget in the end, but that's what our cashflow situation forces us to do).

RachL avatar May 20 '24 07:05 RachL

ok that sounds reasonable. now @chahmedejaz is on #12398. after that, we already agreed he'll investigate if it's easy enough to reuse the report dropdown.

thanks!

mariocarabotta avatar May 21 '24 00:05 mariocarabotta

I'm going to try the spike now to see which path to take.

dacook avatar May 29 '24 03:05 dacook

Ok, I got a proof of concept going. It's actually not too bad, although there's a slight delay after page load before the preferred columns are shown/hidden.

Remaining:

  1. need to update a couple of colspans in _table partial with angular. ❌ I tried this but couldn't work it out.
  2. re-init after turbo frame load (eg after save changes)
  3. move dropdown to desired position (to the right of page size dropdown)
  4. Use a different controller instead of breaking the current AdminProductEditCtrl (can we use ColumnsDropdownCtrl)
  5. Optional: re-style as per design

https://github.com/openfoodfoundation/openfoodnetwork/compare/master...dacook:openfoodnetwork:buu/change-columns-11055-angular Screenshot 2024-05-29 at 1 59 04 pm

~~Given it's pretty close, I think it's worth proceeding. I'll timebox another 30mins to see if I can do items 1-2 to further prove the concept.~~

I tried, failed, and am less happy with how it looks now. I'm going to pause on this for the moment and reconsider.

dacook avatar May 29 '24 04:05 dacook

Question: Do we want to keep user column preferences from the old screen and bring them to the new screen?

So far, I've built it so that the preferences from each screen are separate.

I think it won't be simple to share the column prefs between both screens because there are some differences (different order, and some different columns). I can see a couple of ways to do it:

  • add a layer in column_preferences_controller to load preferences from the old screen, if no other preferences set yet
  • a one-off migration to copy column preferences from old screen to new.

Hmm.. I think it's not worth it, so will make the call and won't raise it further. That is, column preferences from the old screen will not be brought to the new screen.

dacook avatar Jun 12 '24 02:06 dacook

I'd say not worth it too..it's a few clicks away for people, not a big deal

mariocarabotta avatar Jun 13 '24 00:06 mariocarabotta