Skip to content

Commit efe9d27

Browse files
suejung-sentrysuejung-sentry
committed
cleanup scroll
1 parent 772f6d8 commit efe9d27

File tree

2 files changed

+124
-185
lines changed

2 files changed

+124
-185
lines changed

‎static/app/views/prevent/preventAI/manageReposPanel.spec.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ describe('ManageReposPanel', () => {
4242

4343
const mockRepo: Repository = {
4444
id: 'repo-1',
45-
name: 'repo-1',
45+
name: 'org-1/repo-1',
4646
url: 'https://github.com/org-1/repo-1',
4747
provider: {
4848
id: 'integrations:github',
@@ -56,8 +56,8 @@ describe('ManageReposPanel', () => {
5656
};
5757

5858
const mockAllRepos = [
59-
{id: 'repo-1', name: 'repo-1'},
60-
{id: 'repo-2', name: 'repo-2'},
59+
{id: 'repo-1', name: 'org-1/repo-1'},
60+
{id: 'repo-2', name: 'org-1/repo-2'},
6161
];
6262

6363
const defaultProps: ManageReposPanelProps = {

‎static/app/views/prevent/preventAI/manageReposToolbar.tsx‎

Lines changed: 121 additions & 182 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Fragment, useCallback, useEffect, useMemo, useRef, useState} from 'react';
1+
import {useCallback, useEffect, useMemo, useRef, useState} from 'react';
22
import uniqBy from 'lodash/uniqBy';
33

44
import {CompactSelect} from 'sentry/components/core/compactSelect';
@@ -31,65 +31,94 @@ function ManageReposToolbar({
3131
const [searchValue, setSearchValue] = useState<string | undefined>();
3232
const debouncedSearch = useDebouncedValue(searchValue, 300);
3333

34+
const {data, hasNextPage, isFetchingNextPage, isLoading, fetchNextPage} =
35+
useInfiniteRepositories({
36+
integrationId: selectedOrg,
37+
searchTerm: debouncedSearch,
38+
});
39+
40+
const scrollParentRef = useRef<HTMLElement | null>(null);
41+
const scrollListenerIdRef = useRef(0);
42+
const hasNextPageRef = useRef(hasNextPage);
43+
const isFetchingNextPageRef = useRef(isFetchingNextPage);
44+
const fetchNextPageRef = useRef(fetchNextPage);
45+
46+
useEffect(() => {
47+
hasNextPageRef.current = hasNextPage;
48+
isFetchingNextPageRef.current = isFetchingNextPage;
49+
fetchNextPageRef.current = fetchNextPage;
50+
}, [hasNextPage, isFetchingNextPage, fetchNextPage]);
51+
52+
const handleScroll = useCallback(() => {
53+
const el = scrollParentRef.current;
54+
if (!el) return;
55+
if (!hasNextPageRef.current || isFetchingNextPageRef.current) return;
56+
const distanceFromBottom = el.scrollHeight - el.scrollTop - el.clientHeight;
57+
if (distanceFromBottom < 100) fetchNextPageRef.current();
58+
}, []);
59+
60+
const handleRepoDropdownOpenChange = useCallback(
61+
(isOpen: boolean) => {
62+
if (isOpen) {
63+
scrollListenerIdRef.current += 1;
64+
const currentId = scrollListenerIdRef.current;
65+
66+
const attachListener = (attempts = 0) => {
67+
if (scrollListenerIdRef.current !== currentId || attempts > 10) return;
68+
const dropdownLists = document.querySelectorAll('ul[role="listbox"]');
69+
const lastList = dropdownLists[dropdownLists.length - 1];
70+
if (lastList instanceof HTMLElement) {
71+
scrollParentRef.current = lastList;
72+
lastList.addEventListener('scroll', handleScroll, {passive: true});
73+
} else {
74+
setTimeout(() => attachListener(attempts + 1), 20);
75+
}
76+
};
77+
attachListener();
78+
} else if (scrollParentRef.current) {
79+
scrollParentRef.current.removeEventListener('scroll', handleScroll);
80+
scrollParentRef.current = null;
81+
}
82+
},
83+
[handleScroll]
84+
);
85+
86+
useEffect(
87+
() => () => {
88+
if (scrollParentRef.current) {
89+
scrollParentRef.current.removeEventListener('scroll', handleScroll);
90+
}
91+
},
92+
[handleScroll]
93+
);
94+
3495
const organizationOptions = useMemo(
3596
() =>
36-
integratedOrgs?.map(org => ({
97+
(integratedOrgs ?? []).map(org => ({
3798
value: org.id,
3899
label: org.name,
39-
})) ?? [],
100+
})),
40101
[integratedOrgs]
41102
);
42103

43-
const {data, hasNextPage, isFetchingNextPage, isLoading, fetchNextPage} =
44-
useInfiniteRepositories({
45-
integrationId: selectedOrg,
46-
searchTerm: debouncedSearch,
47-
});
48-
49104
const allReposData = useMemo(
50105
() => uniqBy(data?.pages.flatMap(result => result[0]) ?? [], 'id'),
51106
[data?.pages]
52107
);
53-
54-
// Filter out repos where search only matches org name, not repo name
55-
const reposData = useMemo(() => {
56-
if (!debouncedSearch) {
57-
return allReposData;
58-
}
59-
return allReposData.filter(repo => {
60-
const repoName = getRepoNameWithoutOrg(repo.name);
61-
return repoName.toLowerCase().includes(debouncedSearch.toLowerCase());
62-
});
108+
const filteredReposData = useMemo(() => {
109+
if (!debouncedSearch) return allReposData;
110+
const search = debouncedSearch.toLowerCase();
111+
return allReposData.filter(repo =>
112+
getRepoNameWithoutOrg(repo.name).toLowerCase().includes(search)
113+
);
63114
}, [allReposData, debouncedSearch]);
64115

65-
// Auto-fetch more pages if filtering reduced visible results below threshold
66-
useEffect(() => {
67-
const MIN_VISIBLE_RESULTS = 50;
68-
if (
69-
debouncedSearch &&
70-
reposData.length < MIN_VISIBLE_RESULTS &&
71-
hasNextPage &&
72-
!isFetchingNextPage &&
73-
!isLoading
74-
) {
75-
fetchNextPage();
76-
}
77-
}, [
78-
debouncedSearch,
79-
reposData.length,
80-
hasNextPage,
81-
isFetchingNextPage,
82-
isLoading,
83-
fetchNextPage,
84-
]);
85-
86116
const repositoryOptions = useMemo(() => {
87-
let repoOptions = reposData.map(repo => ({
117+
let repoOptions = filteredReposData.map(repo => ({
88118
value: repo.id,
89119
label: getRepoNameWithoutOrg(repo.name),
90120
}));
91121

92-
// Ensure selected repo is always in options even if not in current filtered list
93122
if (selectedRepoData && selectedRepo !== ALL_REPOS_VALUE) {
94123
repoOptions = [
95124
{
@@ -100,154 +129,64 @@ function ManageReposToolbar({
100129
];
101130
}
102131

103-
// Deduplicate by value to prevent React key conflicts
104-
const uniqueRepoOptions = uniqBy(repoOptions, 'value');
132+
const dedupedRepoOptions = uniqBy(repoOptions, 'value');
133+
return [{value: ALL_REPOS_VALUE, label: t('All Repos')}, ...dedupedRepoOptions];
134+
}, [filteredReposData, selectedRepo, selectedRepoData]);
105135

106-
return [{value: ALL_REPOS_VALUE, label: t('All Repos')}, ...uniqueRepoOptions];
107-
}, [reposData, selectedRepo, selectedRepoData]);
108-
109-
function getEmptyMessage() {
110-
if (isLoading) {
111-
return t('Loading repositories...');
112-
}
113-
if (reposData.length === 0) {
136+
const getRepoEmptyMessage = () => {
137+
if (isLoading) return t('Loading repositories...');
138+
if (filteredReposData.length === 0) {
114139
return debouncedSearch
115140
? t('No repositories found. Please enter a different search term.')
116141
: t('No repositories found');
117142
}
118143
return undefined;
119-
}
120-
121-
const scrollListenerRef = useRef<HTMLElement | null>(null);
122-
const scrollListenerIdRef = useRef<number>(0);
123-
124-
const hasNextPageRef = useRef(hasNextPage);
125-
const isFetchingNextPageRef = useRef(isFetchingNextPage);
126-
const fetchNextPageRef = useRef(fetchNextPage);
127-
128-
useEffect(() => {
129-
hasNextPageRef.current = hasNextPage;
130-
isFetchingNextPageRef.current = isFetchingNextPage;
131-
fetchNextPageRef.current = fetchNextPage;
132-
}, [hasNextPage, isFetchingNextPage, fetchNextPage]);
133-
134-
const handleScroll = useCallback(() => {
135-
const listElement = scrollListenerRef.current;
136-
137-
if (!listElement) {
138-
return;
139-
}
140-
141-
// Check if user has scrolled near the bottom
142-
const scrollTop = listElement.scrollTop;
143-
const scrollHeight = listElement.scrollHeight;
144-
const clientHeight = listElement.clientHeight;
145-
const distanceFromBottom = scrollHeight - scrollTop - clientHeight;
146-
147-
if (!hasNextPageRef.current || isFetchingNextPageRef.current) {
148-
return;
149-
}
150-
151-
// Trigger when within 100px of bottom
152-
if (distanceFromBottom < 100) {
153-
fetchNextPageRef.current();
154-
}
155-
}, []);
156-
157-
// Set up scroll listener when menu opens
158-
const handleMenuOpenChange = useCallback(
159-
(isOpen: boolean) => {
160-
if (isOpen) {
161-
// Increment ID to track this specific open instance
162-
scrollListenerIdRef.current += 1;
163-
const currentId = scrollListenerIdRef.current;
164-
165-
// Try multiple times to find the list element as it may take time to render
166-
const tryAttachListener = (attempts = 0) => {
167-
// Stop if menu was closed (ID changed) or too many attempts
168-
if (scrollListenerIdRef.current !== currentId || attempts > 10) {
169-
return;
170-
}
171-
172-
// Find all listbox elements and get the last one (most recently opened)
173-
const listElements = document.querySelectorAll('ul[role="listbox"]');
174-
const listElement = listElements[listElements.length - 1];
175-
176-
if (listElement instanceof HTMLElement) {
177-
scrollListenerRef.current = listElement;
178-
listElement.addEventListener('scroll', handleScroll, {passive: true});
179-
} else {
180-
// Retry after a short delay
181-
setTimeout(() => tryAttachListener(attempts + 1), 20);
182-
}
183-
};
184-
185-
tryAttachListener();
186-
} else {
187-
// Clean up listener when menu closes
188-
if (scrollListenerRef.current) {
189-
scrollListenerRef.current.removeEventListener('scroll', handleScroll);
190-
scrollListenerRef.current = null;
191-
}
192-
}
193-
},
194-
[handleScroll]
195-
);
196-
197-
// Cleanup on unmount
198-
useEffect(() => {
199-
return () => {
200-
if (scrollListenerRef.current) {
201-
scrollListenerRef.current.removeEventListener('scroll', handleScroll);
202-
}
203-
};
204-
}, [handleScroll]);
144+
};
205145

206146
return (
207-
<Fragment>
208-
<PageFilterBar condensed>
209-
<CompactSelect
210-
value={selectedOrg}
211-
options={organizationOptions}
212-
onChange={option => onOrgChange(option?.value ?? '')}
213-
triggerProps={{
214-
icon: <IconBuilding />,
215-
children: (
216-
<TriggerLabel>
217-
{organizationOptions.find(opt => opt.value === selectedOrg)?.label ||
218-
t('Select organization')}
219-
</TriggerLabel>
220-
),
221-
}}
222-
/>
223-
224-
<CompactSelect
225-
value={selectedRepo}
226-
options={repositoryOptions}
227-
loading={isLoading}
228-
disabled={!selectedOrg || isLoading}
229-
onChange={option => onRepoChange(option?.value ?? '')}
230-
searchable
231-
disableSearchFilter
232-
onSearch={setSearchValue}
233-
searchPlaceholder={t('search by repository name')}
234-
onOpenChange={isOpen => {
235-
setSearchValue(undefined);
236-
handleMenuOpenChange(isOpen);
237-
}}
238-
emptyMessage={getEmptyMessage()}
239-
triggerProps={{
240-
icon: <IconRepository />,
241-
children: (
242-
<TriggerLabel>
243-
{repositoryOptions.find(opt => opt.value === selectedRepo)?.label ||
244-
t('Select repository')}
245-
</TriggerLabel>
246-
),
247-
}}
248-
/>
249-
</PageFilterBar>
250-
</Fragment>
147+
<PageFilterBar condensed>
148+
<CompactSelect
149+
value={selectedOrg}
150+
options={organizationOptions}
151+
onChange={opt => onOrgChange(opt?.value ?? '')}
152+
menuWidth="200px"
153+
triggerProps={{
154+
icon: <IconBuilding />,
155+
children: (
156+
<TriggerLabel>
157+
{organizationOptions.find(opt => opt.value === selectedOrg)?.label ||
158+
t('Select organization')}
159+
</TriggerLabel>
160+
),
161+
}}
162+
/>
163+
<CompactSelect
164+
value={selectedRepo}
165+
options={repositoryOptions}
166+
loading={isLoading}
167+
disabled={!selectedOrg || isLoading}
168+
onChange={opt => onRepoChange(opt?.value ?? '')}
169+
searchable
170+
disableSearchFilter
171+
onSearch={setSearchValue}
172+
searchPlaceholder={t('search by repository name')}
173+
onOpenChange={isOpen => {
174+
handleRepoDropdownOpenChange(isOpen);
175+
if (!isOpen) setSearchValue(undefined);
176+
}}
177+
emptyMessage={getRepoEmptyMessage()}
178+
menuWidth="250px"
179+
triggerProps={{
180+
icon: <IconRepository />,
181+
children: (
182+
<TriggerLabel>
183+
{repositoryOptions.find(opt => opt.value === selectedRepo)?.label ||
184+
t('Select repository')}
185+
</TriggerLabel>
186+
),
187+
}}
188+
/>
189+
</PageFilterBar>
251190
);
252191
}
253192

0 commit comments

Comments
 (0)