-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fire Weather Map Updates #585
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Noticed two issues that I've noted in code review that we should look into before merging.
Additionally, the page heading is an h2
, but it should be an h1
. I'm unsure where that's getting set (I assume it's by the Plot?).
const query = gql` | ||
query FireWeatherMap { | ||
fireDangerMapCollection(limit: 1) { | ||
items { | ||
description { | ||
json | ||
} | ||
} | ||
} | ||
} | ||
`; |
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 could have an adverse side-effect if there's more than one entry erroneously created; I'd recommend doing something like we did on /licensing-permits
where we look up the pageMetadata ID based on the route (and then query for the one entry instead of a collection):
const metadataID = pathsToIDs.get(`/licensing-permits`); |
src/routes/(infoPages)/land/fire/safety/fire-danger-map/+page.svelte
Outdated
Show resolved
Hide resolved
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.
Pre-approving so I don't block you, but yeah I think at the least we need to handle the issue with the heading and the labels that appear on the parishes before merging.
…her map page. (#586) Signed-off-by: Louis Fettet <[email protected]>
Coverage after merging dh/fire-weather-map-updates into main will be
Coverage Report
|
Reports for 8d5e837 have been deployed to Vercel: |
Proposed changes
Screenshots
Notice that it gets included in the navigation side bar:
![Screenshot 2024-10-31 at 9 49 10 PM](https://private-user-images.githubusercontent.com/13280995/382168838-6036f49d-895a-4fcb-b09c-44ba81d3472f.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDgyODksIm5iZiI6MTczOTY0Nzk4OSwicGF0aCI6Ii8xMzI4MDk5NS8zODIxNjg4MzgtNjAzNmY0OWQtODk1YS00ZmNiLWIwOWMtNDRiYTgxZDM0NzJmLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE5MzMwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWYzMDQwY2JmZDg0Yzg3YjExOGFmMmM1YjFkOTc4NTBjOGEwZDQ2NmJjYzJiMGM3MTVkNmU5YWJjZjI4NDhiZTkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.wWrFM0edkLUWQzt6bGZ_cpMhP0cn6g_-D34CVQvXZxs)
Description rich text served from Contentful:
![Screenshot 2024-10-31 at 9 49 20 PM](https://private-user-images.githubusercontent.com/13280995/382168885-b3f46489-b0c0-4463-aa80-b51b4084fb39.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3Mzk2NDgyODksIm5iZiI6MTczOTY0Nzk4OSwicGF0aCI6Ii8xMzI4MDk5NS8zODIxNjg4ODUtYjNmNDY0ODktYjBjMC00NDYzLWFhODAtYjUxYjQwODRmYjM5LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNTAyMTUlMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjUwMjE1VDE5MzMwOVomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTNlMWUxNWUwYmY1Mjg1NzdlNjNhNjMzNWM0OTVlNWU2NTU1NDQ3ZmUzNjcwYjg1MTFhMzFhYWUxZDE4ZjM3ZWYmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.RtnonZ3i_M8CxMKJOxD7XBCwypVNxTOgiTqE46hOH10)
Acceptance criteria validation
Other details
Alternate solutions
Possible drawbacks
Requested feedback