-
Notifications
You must be signed in to change notification settings - Fork 16
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
[Issue #1658]: Agency filter list #1772
Conversation
@andycochran @crystabelrangel - let me know what labels we want to manually change |
Can you swap the square brackets for parentheses, like below? We'll need to change any of these that don't match. These are the only ones that were reviewed. @crystabelrangel, do you remember which ones we changed during approvals? "ED" came to mind, but it looks like it's already right in the data. |
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.
AgencyNameLookup
?
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.
…since it's also used for the results, not just for the filter?
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.
changed to generateAgencyNameLookup
and agencyNameLookup
Also changed to parentheses.
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.
Can you nix the prefix "All"? We think it's unnecessary and implied by the nesting/dropdown (and will test) |
opportunity?.summary?.agency_code && | ||
agencyNameLookup | ||
? // Use same exact label we're using for the agency filter list | ||
agencyNameLookup[opportunity?.summary?.agency_code] |
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.
@andycochran - this is the key part here - make sure this is what you want. We're displaying the exact checkbox choice label (if it's actually in the agencyNameLookup
)
@andycochran I've removed the |
Summary
Fixes #1658
Time to review: 5 min
Changes proposed
SearchResultsListItem
that the agency filters useContext for reviewers