-
Notifications
You must be signed in to change notification settings - Fork 1k
[hackaton] solar assets layer #8161
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
base: master
Are you sure you want to change the base?
Conversation
}); | ||
if (solarAssetFeature) { | ||
console.log('[Map.tsx onMouseMove] Solar asset detected:', solarAssetFeature); | ||
const properties = (solarAssetFeature.properties as Record<string, any>) || {}; |
Check warning
Code scanning / ESLint
Disallow the `any` type Warning
Copilot Autofix
AI 1 day ago
Copilot could not generate an autofix suggestion
Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.
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.
Long review and a lot of comments, sorry about that but it's a big PR...
{selectedSolarAsset ? ( | ||
<div className="pointer-events-none absolute inset-0 z-10 sm:flex sm:w-[calc(14vw_+_16rem)]"> | ||
<Suspense> | ||
<SolarAssetPanel /> | ||
</Suspense> | ||
</div> | ||
) : ( | ||
<Suspense> | ||
<LeftPanel /> | ||
</Suspense> | ||
)} |
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.
To reduce duplicated code this should probably live inside the LeftPanel so the header and navigation can be re-used.
|
||
const getSolarAssets = async (): Promise<any> => { | ||
const path: URL = new URL( | ||
'https://storage.googleapis.com/testing-gzipped-geojson/solar_power_plants.geojson' |
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 should probably live behind our own domain, we can set up a simple proxy in Cloudflare to ensure it's properly cached and we get all the metrics. It has default support for buckets and such.
export const getFilteredSolarAssets = ( | ||
searchTerm: string, | ||
solarAssets: SolarAssetRowType[] | ||
): SolarAssetRowType[] => { | ||
if (!searchTerm) { | ||
return []; | ||
} | ||
|
||
const searchLower = searchTerm.toLowerCase(); | ||
|
||
return solarAssets.filter( | ||
(asset) => | ||
asset.name.toLowerCase().includes(searchLower) || | ||
(asset.country && asset.country.toLowerCase().includes(searchLower)) || | ||
(asset.capacity && asset.capacity.toLowerCase().includes(searchLower)) || | ||
(asset.status && asset.status.toLowerCase().includes(searchLower)) | ||
); | ||
}; | ||
|
||
// Combined search function that returns both zone and solar asset results | ||
export const getCombinedSearchResults = ( | ||
searchTerm: string, | ||
zoneData: Record<string, ZoneRowType>, | ||
solarAssets: SolarAssetRowType[] | ||
): SearchResultType[] => { | ||
if (!searchTerm) { | ||
return []; | ||
} | ||
|
||
const filteredZones = getFilteredList(searchTerm, zoneData); | ||
const filteredSolarAssets = getFilteredSolarAssets(searchTerm, solarAssets); | ||
|
||
// Combine results - zones first, then solar assets | ||
return [...filteredZones, ...filteredSolarAssets]; | ||
}; |
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.
The above function should just be updated to include these directly as we are duplicating logic and doing work twice now, as well as unpacking and re-packing two potentially large arrays. If we add more assets like wind turbines in the future this is going to be a real problem.
Co-authored-by: Viktor Andersson <30777521+VIKTORVAV99@users.noreply.github.com>
This reverts commit d6676e9.
I've tackled the easier parts of this, the still unresolved stuff seems trickier. |
This reverts commit 45bae7d.
No description provided.