City Filter Function#3983
Conversation
|
@SipheDilima247 is attempting to deploy a commit to the World Monitor Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR adds a city search feature to the CMD+K search modal and introduces a
Confidence Score: 3/5The city search integration is straightforward and safe to merge; the country profile feature has multiple functional defects that prevent it from working correctly and will degrade DOM event handling over time. The CountrySelector modal will never be visible because its container is never inserted into the document. Both CountrySelector and CountryProfileView register listeners on document with no cleanup path, so each open/close cycle silently accumulates handlers. These three issues affect the new country profile feature end-to-end. The city search path itself is clean, but it ships in the same PR. src/app/country-profile-manager.ts, src/components/CountrySelector.ts, and src/components/CountryProfileView.ts all need attention before the country profile feature can work correctly. Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant SearchModal
participant SearchManager
participant Map
participant App
participant CountryProfileManager
participant CountrySelector
participant CountryProfileView
User->>SearchModal: types city name
SearchModal->>SearchManager: "result selected (type='city')"
SearchManager->>Map: setView('global')
SearchManager->>Map: "setCenter(lat, lng, zoom=5) [after 300ms]"
User->>App: openCountrySelector()
App->>CountryProfileManager: openCountrySelector()
CountryProfileManager->>CountrySelector: "new CountrySelector({container: selectorContainer})"
Note over CountrySelector: selectorContainer not in DOM — invisible
CountrySelector-->>CountryProfileManager: onCountrySelected(code, name)
CountryProfileManager->>CountryProfileView: new CountryProfileView(...)
CountryProfileView->>document: addEventListener('keydown', keyHandler)
Note over CountryProfileView: keyHandler never removed on destroy()
CountryProfileView-->>App: renders overlay
User->>CountryProfileView: closes profile
CountryProfileView->>CountryProfileManager: onClose()
CountryProfileManager->>CountryProfileView: destroy()
Note over document: orphaned keydown listener remains
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| /** | ||
| * Opens the country selector modal for the user to choose a country | ||
| */ | ||
| public openCountrySelector(): void { | ||
| if (this.countrySelector) { |
There was a problem hiding this comment.
selectorContainer never appended to the DOM — CountrySelector is invisible
this.selectorContainer is created with document.createElement('div') and assigned an id, but it is never inserted into the document tree. CountrySelector receives it as options.container, skips the fallback document.body.appendChild(this.container), and renders its .country-selector wrapper inside a detached node. Even though .country-selector is position: fixed, it still needs an ancestor in the live document to be painted. openCountrySelector() therefore produces a functional no-op — the modal is never visible to the user.
|
|
||
| return title.includes(countryCode.toLowerCase()) || | ||
| title.includes(countryCode.toUpperCase()) || | ||
| countries.includes(countryCode.toLowerCase()); | ||
| }); | ||
|
|
||
| // Store filtered news for use in panels | ||
| (window as any).__COUNTRY_PROFILE_NEWS = countryRelevantNews; | ||
| } | ||
|
|
||
| /** | ||
| * Loads country-specific panel data (uses CountryDeepDivePanel) | ||
| */ | ||
| private loadCountryPanelData(countryCode: string, countryName: string): void { | ||
| if (!this.countryProfileView) return; |
There was a problem hiding this comment.
title.includes(countryCode.toLowerCase()) checks for the 2-letter ISO code as a substring in the already-lowercased title string. Codes like "US" → "us", "IN" → "in", "IR" → "ir", or "DE" → "de" appear in thousands of common English words (discuss, include, inside, index, decide, etc.), so virtually every news item would match. The second check title.includes(countryCode.toUpperCase()) is also dead code because title was already converted to lowercase and will never contain uppercase characters.
| this.ctx.searchModal.registerSource('city', getCitySearchItems()); | ||
| } | ||
|
|
||
| private buildCountrySearchItems(): { id: string; title: string; subtitle: string; data: { code: string; name: string } }[] { |
There was a problem hiding this comment.
registerSource('city', ...) call in updateSearchIndex()
City coordinates are a static import from CITY_COORDS and never change at runtime. The initial registration at setup time already covers it. Re-calling getCitySearchItems() and re-registering on every dynamic index update is unnecessary work. The call should be removed from updateSearchIndex().
| this.ctx.searchModal.registerSource('city', getCitySearchItems()); | |
| } | |
| private buildCountrySearchItems(): { id: string; title: string; subtitle: string; data: { code: string; name: string } }[] { | |
| } | |
| private buildCountrySearchItems(): { id: string; title: string; subtitle: string; data: { code: string; name: string } }[] { |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Did you see all the comments greptile wrote on your PR @SipheDilima247 |
2 participants
Summary
Type of change
Affected areas
/api/*)Checklist
api/rss-proxy.jsallowlist (if adding feeds)npm run typecheck)