ref(projectfilters) move checkbox to left#108090
ref(projectfilters) move checkbox to left#108090JonasBa merged 12 commits intojb/projectfilters/searchfrom
Conversation
github-actions
bot
added
the
Scope: Frontend
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- Add checkboxes to EnvironmentPageFilter, FilterSelector, and TestSuiteDropdown - Fix project filter search by adding textValue property (was broken due to JSX label) - Fix menu width calculation to use textValue instead of stringifying JSX label - Use HybridFilterRef with imperative handle pattern for all filters
Updates test to include leadingItems with checkboxes for each filter option, matching the new implementation pattern after HybridFilter refactoring.
| map.set(value, { | ||
| label: middleEllipsis(value, 70, /[\s-_:]/), | ||
| value, | ||
| leadingItems: ({isSelected}: {isSelected: boolean}) => ( |
There was a problem hiding this comment.
The reason we have to set this manually is because the hybrid filter no longer manages it for us. As it turns out, hybridFilter has a "hideCheck" option that removes the checkbox from compactSelect, which is how it was able to hide it, and then abuse trailingItems or leadingItems to toggle where the rendering of it was supposed to be...
There was a problem hiding this comment.
hybridFilter should not have ever been exposed as a component outside of pagefilters, I am hoping that the prevent instance can soon be removed, and that we can do something about the dashboard case, but it requires more investigation
There was a problem hiding this comment.
we talked about this briefly in slack, but why can’t we just get rid of hideCheck completely and just use the built-in checkbox from compactSelect ?
compactSelect already has a way to have multi-selects “stay open” after a selection has been made, so I guess the main problem is that selecting one value in the checkbox immediately triggers onChange and updates the url and everything.
Maybe this should be a built-in feature of compactSelect, or maybe even all compactSelects with multiple: true should behave that way? Isn’t it weird that clicking a checkbox sometimes immediately applies and sometimes not, like e.g. here:
Teams and AlertType instantly apply, but Projects does not. This isn’t great in terms of “least surprise” ...
Screen.Recording.2026-02-12.at.09.19.28.mov
If we’d unify this, I think using what the ProjectsFilter does right now for everything (built into compactSelect) wouldn’t be bad:
- you click the label, it instantly applies
- you click the checkbox or cmd-click the label, it selects but doesn’t apply and shows the
Apply/Cancelin the footer andresetin the header.
It would be a change in UX for some places but arguably for the better.
@Jesse-Box FYI
There was a problem hiding this comment.
@TkDodo Your proposal makes perfect sense. This change visually aligns hybridSelects closer to what other compactSelects look like, so it's working towards that.
@natemoo-re has been doing a menu component audit across the codebase, and I'd like to followup to that by building this behavior directly into compactSelect because I agree, it's a very unpredictable UX
There was a problem hiding this comment.
Awesome. Approving so we can get this merged and the unification can happen afterwards
The pagefilter checkboxes were moved to the left, and the project/env labels are now nested inside Flex/Text components. This updates the XPath selectors to search within descendant nodes instead of direct text children.
github-actions
bot
added
the
Scope: Backend
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
|
Had to update acceptance tests to fix the xpath selector which broke with the DOM changes |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| return ( | ||
| <Button | ||
| title={label} | ||
| aria-label={label} |
There was a problem hiding this comment.
Parent tooltipProps overrides component's optimistic tooltip state
Low Severity
Switching from title to tooltipProps introduces a regression where the {...props} spread on the Button now allows the parent's tooltipProps to override the component's internal tooltipProps. In projectPageFilter.tsx, BookmarkStar is called with tooltipProps={{title: project.isBookmarked ? ...}} using the stale project.isBookmarked prop, which now overrides the component's own tooltipProps that correctly uses the local optimistic isBookmarked state. After toggling a bookmark, the tooltip briefly shows the wrong text until project data refreshes.
Additional Locations (1)
2 participants
Moves checkbox position to the beginning of the row and unwinds the mess of checkwraps.
Fix DE-752