feat: Rewrite partner filter #2561
Conversation
| <div class="preview__details"> | ||
| <%= content_tag :p, description %> | ||
| <%= content_tag :p, @partner.summary %> | ||
| <!-- Categories: <%= @partner.categories.map(&:name).join(", ") %> --> |
There was a problem hiding this comment.
Leaving this in for debugging purposes in the future
…ange/PlaceCal into refactor-partner-filter
|
added a turbo frame - speeds up performance significantly by only updating the changed content Screen.Recording.2024-08-19.at.14.52.09.mov |
|
Unfortunately I had to take out turbo loading as I can't work out how to get the filter options to reload properly when selected, so I'm leaving that for another ticket - this one is already big enough. Leaving in how to do it commented out as it nearly works I just can't get it over the line. I've tidied up the CSS, made it set the width dynamically, and hopefully made it easier to read. The width between the two is not dynamically set though, so future improvement for someone. 
|
|
All working! Original post updated with info about status. I would like to merge this now and then start tackling some of the structural problems, like the state of the components folder, before improving this further. |
| this.submitForm(); | ||
| } | ||
|
|
||
| toggleCategory() { |
There was a problem hiding this comment.
this should be spun into its own stimulus controller but waiting on sorting the components directory out first
| @@ -1,16 +1,18 @@ | |||
| <li class="preview"> | |||
| <div class="preview__header"> | |||
| <h3><%= link_to name, link %></h3> | |||
| <% if show_service_area? || show_neighbourhood? %> | |||
There was a problem hiding this comment.
Just checking since the code is a little scattered - we've not removed showing of service area it's just that it is now incorporated into the neighbourhood property?
There was a problem hiding this comment.
a partner can have a neighbourhood 2 ways: either because of its address, or because of its service area :)
i think for sure the partner.rb file should do a better job of explaning its API
There was a problem hiding this comment.
Is there a setting that this is checking on the partner - for whether they want their address / service area to be visible?
There was a problem hiding this comment.
i think it prioritises service area if set and address if not, assumption is that if a service area is set, its not being delivered at the address. again - we should have cucumber tests for this.
|
|
||
| updateLabels() { | ||
| // Find the associated label for each selected param and get the text contents | ||
| // If params are selected, they show up instead of "Category" and "Neighbourhood" text |
There was a problem hiding this comment.
I found this confusing in the UI. but I guess it's a way to show something is selected and can only select one thing at a time from each list now, right?
There was a problem hiding this comment.
yes - i think we need to replace this whole thing with a <select> box - its in marks' new design but obvi getting v out of scope
There was a problem hiding this comment.
Agree. Do we bother making sure there's an issue for future us? Or will it naturally arise?
There was a problem hiding this comment.
i think its covered under #1812
|
working on staging Screen.Recording.2024-09-02.at.19.01.46.mov |
Rewritten Partner filter using Stimulus and view_component.
Screen.Recording.2024-08-22.at.16.01.18.mov
Progress
I have removed the exclude functionality as it's too confusing and wasn't working properly anyway.
Left to do
Future improvements
Fixes #2544
Fixes #2081
Fixes #2087
Fixes #1780
Fixes #1906