- Issue created by @phenaproxima
- First commit to issue fork.
- Merge request !699#3502935: Refactor the Search component to treat filter changes generically → (Merged) created by utkarsh_33
- 🇮🇳India utkarsh_33
The tests are all green which means functionally everything is working as expected.I have a done some basic restructuring for now (which might be silly) but i think it's a good start for now.I'll ask @phenaproxima to guide me more on this with what i need to follow in order to achieve the goal.
- 🇮🇳India utkarsh_33
I will discuss this with @phenaproxima and update the status of the issue soon.
- 🇮🇳India utkarsh_33
I think this is now ready and changes are inline with the discussion that i had with @phenaproxima.Marking it NR .
- 🇮🇳India narendraR Jaipur, India
Tested manually and it seems that On selecting multiple categories, it does not announce the Count of selected categories when voiceover is on.
- 🇺🇸United States phenaproxima Massachusetts
This generally looks good. I think we can remove
filtersWrapper
, it seems like a pointless extra layer around what$filters
is already doing.We do need to fix the announce problem that @narendrar found. I would suggest that we change the backend
MultipleChoiceFilter
class so that it accepts a "template string" that can be used for announcements, and then have the announce function in Svelte use that. Make it part of the filter's definition -- how does it want to announce a state change to the user? All the filters should be taking this sort of accessibility into account, but let's fix it for the MultipleChoiceFilter type first, and the other filter types later. - 🇺🇸United States phenaproxima Massachusetts
This is making a ton of sense. There are a few more steps to go, but this is really solid so far!
- 🇮🇳India utkarsh_33
I am facing issues in solving the test fails.I checked the URLs formed in both 2.0.x and this branch are same.
- 🇺🇸United States phenaproxima Massachusetts
Turns out the tests were only failing because we weren't always passing the filters to
load()
inProjectBrowser.svelte
. That makes sense -- we probably always want to pass them explicitly. - First commit to issue fork.
-
chrisfromredfin →
committed d1bb6215 on 2.0.x authored by
utkarsh_33 →
Issue #3502935: Refactor the Search component to treat filter changes...
-
chrisfromredfin →
committed d1bb6215 on 2.0.x authored by
utkarsh_33 →
- 🇺🇸United States chrisfromredfin Portland, Maine
I think with automated tests passing for a known reason, and the code changes being grokable, and me manual testing, I think this one looks good!
Automatically closed - issue fixed for 2 weeks with no activity.