Feature/#53 Layout Reservoirs by province #59
Conversation
monikMononoke
changed the title
There was a problem hiding this comment.
Pull request overview
This PR implements feature #53, adding a page that displays a list of Spanish provinces with links to view reservoirs by province. The implementation replaces a hardcoded link to Málaga with a dynamic list generated from an array of all 50 Spanish provinces.
Key Changes
- Created a new array containing all 50 Spanish provinces
- Updated the province listing page to dynamically render links for all provinces
- Added styling to the page with rounded corners and padding
- Updated package-lock.json with platform-specific binaries for build tools (esbuild, rollup, turbo)
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
package-lock.json |
Added platform-specific optional dependencies for esbuild 0.25.9, rollup 4.46.4, Next.js 15.5.0, and turbo 2.5.6 build tools |
front/src/app/embalse-provincia/page.tsx |
Replaced hardcoded Málaga link with dynamic list rendering all provinces, added styling |
front/src/app/embalse-provincia/array-provincias.ts |
New file containing array of 50 Spanish province names |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Link href="/embalse-provincia/malaga" className="link-accessible"> | ||
| Málaga | ||
| </Link> | ||
| <div className="flex flex-col gap-8 rounded-2xl bg-(--color-base-100) p-4"> |
There was a problem hiding this comment.
The CSS class syntax bg-(--color-base-100) is incorrect. Based on the pattern used in other components in this codebase (e.g., footer.component.tsx and header.component.tsx), this should be bg-base-100 instead. The current syntax will not apply the background color correctly.
| <div className="flex flex-col gap-8 rounded-2xl bg-(--color-base-100) p-4"> | |
| <div className="flex flex-col gap-8 rounded-2xl bg-base-100 p-4"> |
| {provincias.map((provincia) => ( | ||
| <Link | ||
| key={provincia} | ||
| href={`/embalse-provincia/${provincia}`} |
There was a problem hiding this comment.
The href uses provincia names directly without URL encoding. Province names contain special characters (accents like 'Á', 'Ó', 'É', spaces like 'Santa Cruz de Tenerife', 'La Coruña', etc.) which need to be properly URL-encoded. Consider using encodeURIComponent(provincia) in the href to ensure the URLs are correctly formed.
| href={`/embalse-provincia/${provincia}`} | |
| href={`/embalse-provincia/${encodeURIComponent(provincia)}`} |
| "Córdoba", | ||
| "La Coruña", | ||
| "Cuenca", | ||
| "Gerona", |
There was a problem hiding this comment.
The province name "Gerona" uses the Spanish (Castilian) form rather than the Catalan official name "Girona". While both are acceptable, consider using the official Catalan form "Girona" for consistency with the constitutional name of the province. The same applies to "Lérida" (line 30) which should be "Lleida".
| "La Rioja", | ||
| "Las Palmas", | ||
| "León", | ||
| "Lérida", |
There was a problem hiding this comment.
The province name "Lérida" uses the Spanish (Castilian) form rather than the Catalan official name "Lleida". While both are acceptable, consider using the official Catalan form "Lleida" for consistency with the constitutional name of the province.
| "Lérida", | |
| "Lleida", |
| "Málaga", | ||
| "Murcia", | ||
| "Navarra", | ||
| "Orense", |
There was a problem hiding this comment.
The province name "Orense" uses the Spanish (Castilian) form rather than the Galician official name "Ourense". While both are acceptable, consider using the official Galician form "Ourense" for consistency with the constitutional name of the province.
| "Orense", | |
| "Ourense", |
Closing issue #53