-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
ref(shared-views): Refactor feature flags and remove some more old components #91506
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
Conversation
@@ -25,10 +25,10 @@ function IssueListFilters({query, sort, onSortChange, onSearch}: Props) { | |||
const organization = useOrganization(); | |||
|
|||
const hasIssueViews = organization.features.includes('issue-stream-custom-views'); | |||
const hasIssueViewSharing = useHasIssueViewSharing(); | |||
const prefersStackedNav = usePrefersStackedNav(); |
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.
Here, and in severla other places, we used to check if the user had issue view sharing enabled. Since issue view sharing flag has been completely swallowed by the stacked nav flag, we can replace it with the usePrefersStackedNav() hook.
groupSearchView && | ||
canEditIssueView({groupSearchView, user, organization}) | ||
) { | ||
if (groupSearchView && canEditIssueView({groupSearchView, user, organization})) { |
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 component (PageTitle) is already gated by the stacked nav hook in a parent component (LeftNavViewsHeader)
viewIds: newViews | ||
.filter(view => view.id[0] !== '_' && !view.id.startsWith('default')) | ||
.map(view => parseInt(view.id, 10)), |
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 shenanignas was cuz of the old default views and the "uncomitted" views. We can remove it now.
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
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.
Looks like there are some failing tests still? Otherwise looks good
Removes some components that are no longer being used as part of the issue view sharing release.
Also refactors some feature flag checks