lib-classifier: Refactor survey task filtering #6619
Conversation
…ove no longer applicable comments
…ssifier_chooser-styling-updates
…ssifier_chooser-styling-updates
…_survey-task-filters-refactor
mcbouslog
changed the title
| direction='row' | ||
| > | ||
| <SpacedText size='1rem'> | ||
| {t('SurveyTask.CharacteristicsFilter.closeFilters')} |
There was a problem hiding this comment.
Should this have a hover style? It currently has a focus style - box-shadow: 0 0 2px 2px #addde0; - could add the same for hover.
cc: @seanmiller26
| <Button | ||
| key={`${characteristicId}-${selectedValueId}`} | ||
| a11yTitle={t('SurveyTask.CharacteristicsFilter.removeFilter', { valueLabel: label })} | ||
| label={ | ||
| <FilterLabel | ||
| characteristicId={characteristicId} | ||
| selected={true} | ||
| valueId={selectedValueId} | ||
| valueImageSrc={valueImageSrc} | ||
| valueLabel={label} | ||
| /> | ||
| } | ||
| margin={{ | ||
| bottom: 'xxsmall', | ||
| left: 'xxsmall' | ||
| }} | ||
| onClick={clearSelection} | ||
| plain | ||
| /> |
There was a problem hiding this comment.
Should the buttons to remove a filter have a hover style?
Currently there might be a very subtle change in the color of the close button that sits on top of the filter image, but maybe with the new button with the "x" to the right of the image a border or box-shadow on the button might be helpful on hover.
cc: @seanmiller26
| function handleFilterDropOpen () { | ||
| setFilterDropOpen(true) | ||
| } | ||
| const [filterOpen, setFilterOpen] = useState(false) |
There was a problem hiding this comment.
I think adding Grommet's AnnounceContext to announce how many choices ("Showing x of y choices") after applying a filter is straightforward:
const announce = useContext(AnnounceContext)
useEffect(function announceFilteredChoiceCount () {
announce(`Showing ${filteredChoiceIds.length} of ${task.choices.size} choices`, 'polite', 1000)
}, [filteredChoiceIds?.length, task.choices.size, announce])
Not sure if it would be helpful or unhelpful noise? Once a user tabs to the menu item list similar information ("item x of y") is announced, so this might not be change to be made.
There was a problem hiding this comment.
I don't have a definitive opinion on this, but considering it's few lines of code and you thought it worth investigating, happy to leave it in for now unless a volunteer reports difficulty using accessibility tools and the survey task.
…_survey-task-filters-refactor
…_survey-task-filters-refactor
|
The filters are functioning as expected and the Storybook looks good~ However, while testing app-project run locally, I noticed that an expandable filters section introduces an annoying page jump, and it's worth flagging to @seanmiller26. Below is a video showing what happens when I expand the task area, and then collapse the task area. Because that component of the page is increasing and decreasing in height, the page appears to scroll down below the classifier on collapse. I anticipate volunteers will immediately flag this as annoying in comparison to filters that popover. Is there anything we can do mitigate the jumping effect on filters collapse? jumping.mov |
Package
Linked Issue and/or Talk Post
DropButtonwhich these changes remove)Describe your changes
How to Review
Helpful explanations that will make your reviewer happy:
Checklist
PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.
General
yarn panic && yarn bootstrapordocker-compose up --buildand FEM works as expectedGeneral UX
Example Staging Project: i-fancy-cats
Bug Fix
Refactoring
Post-merge