-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
fix(releases): Use query parameter when switching filters #52937
Conversation
There was a large ternary with a mistake setting the current filter to state. We can use the query parameter instead of state.
: query.includes(IssuesType.REGRESSED) | ||
? IssuesType.UNHANDLED |
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.
This was the mistake, but the real mistake was this massive ternary
@@ -56,7 +56,6 @@ type Props = { | |||
location: Location; | |||
organization: Organization; | |||
releaseBounds: ReleaseBounds; | |||
selection: PageFilters; |
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.
unused
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.
Logic is much better now 💯
RESOLVED = 'is:resolved', | ||
ALL = 'release', | ||
} | ||
const IssuesQuery: Record<IssuesType, string> = { |
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.
nit: should this still be pascal case now that it's not an enum?
There was a large ternary with a mistake setting the current filter to the wrong option. We can use the query parameter instead of state altogether.
fixes #52880