-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[STAN-557] Search and Filter fixes #145
Conversation
Maybe an integration test? |
The same bug also affects searching from the header bar on other pages too, so you may wish to also cover that. |
.summary { | ||
border: 10px solid yellow; | ||
&:before { | ||
border: 10px solid lime !important; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ORLY?!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😮💨
// TODO: better sorting needed | ||
const activeFilters = standard_category | ||
? { ...chosenFilters, standard_category } | ||
: chosenFilters; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than forcing the ordering so that category is at the end (but leaving care setting and topic potentially misordered), it feels probably better to just iterate over a fixed set of keys like we do (indirectly) here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, leaving a TODO in my branch to pick this up after merge->deploy->demo etc
""
, bringing back all results)Filter updates
Type
(we write "in" for that)* expander icons aligned with headings
* When clicking "mandated" alone we no longer get two lines across the filter summary section
before
after