openfoodfoundation / openfoodnetwork

Connect suppliers, distributors and consumers to trade local produce.
https://www.openfoodnetwork.org
GNU Affero General Public License v3.0
1.11k stars 718 forks source link

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

Closed mariocarabotta closed 4 months ago

mariocarabotta commented 1 year ago

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
NickIkenma commented 1 year ago

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?

mariocarabotta commented 9 months ago

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 commented 8 months ago

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

dacook commented 8 months ago

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:

mariocarabotta commented 8 months ago

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

dacook commented 6 months ago

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 commented 5 months ago

@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).
  2. 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.

mariocarabotta commented 5 months ago

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?

RachL commented 5 months ago

@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.

mariocarabotta commented 5 months ago

@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.

chahmedejaz commented 5 months ago

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.

mariocarabotta commented 5 months ago

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.

dacook commented 5 months ago

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.

RachL commented 5 months ago

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).

mariocarabotta commented 5 months ago

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!

dacook commented 4 months ago

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

dacook commented 4 months ago

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 commented 4 months ago

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:

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.

mariocarabotta commented 4 months ago

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