Skip to content
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

Feat(climate-tool): Forecast page placeholder #281

Merged
merged 12 commits into from
Jun 18, 2024
Merged

Conversation

khalifan-kfan
Copy link
Collaborator

@khalifan-kfan khalifan-kfan commented May 29, 2024

Description

This pull request adds the page for climate forecast in the climate tool.

Discussion

closes #277

Preview

Screenshots / Videos

Please check the route localhost:4200/climate/forecast to test.

Screenshot 2024-05-29 at 22 37 36

on click (dummy pdf)
Screenshot 2024-05-29 at 22 37 27

@khalifan-kfan khalifan-kfan added the Tool: Climate Updates related to Climate tool label May 29, 2024
@github-actions github-actions bot added feature Tool: Manual Updates related to Manual tool Tool: Resources Updates related to Resources tool labels May 29, 2024
@khalifan-kfan khalifan-kfan marked this pull request as ready for review May 29, 2024 20:03
@github-actions github-actions bot added feature and removed feature labels Jun 4, 2024
Copy link
Collaborator

@chrismclarke chrismclarke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @khalifan-kfan
From what I can see it's looking in a good state (will just need to hook up the live data when available and do some minor tidying), although I noticed this PR also has the partial changes from the resources share PR which still needs a bit of work.

So either it would be better to wait until after that is merged, or cherry-pick the relevant commits for the forecasts feature and put them on a clean branch and open a new pr.

Let me know what approach you would like to take and I can help assist

@khalifan-kfan
Copy link
Collaborator Author

Ohh sorry about that, let me first attempt the cherry-picking and see if it could help

@chrismclarke chrismclarke changed the title Feat: Forecast page placeholder Feat(climate-tool): Forecast page placeholder Jun 18, 2024
@github-actions github-actions bot added feature and removed feature labels Jun 18, 2024
Copy link
Collaborator

@chrismclarke chrismclarke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @khalifan-kfan

All looks good now, thanks for tidying up the commits.

I've added one small commit 3c10a17 to refactor some *ngIf to @if(...) syntax and replace the module with standalone component just because I'm slowly trying to migrate more of the code base to use the modern syntax. I've also fixed an issue where the slide-in animation wasn't showing (basically the parent div needs to always be available with a .active class selector and child content contain the if condition instead)

I've also removed the placeholder forecast for now until we get the feature fully integrated with the climate api, but happy to merge as-is and then add a new issue to complete the feature when ready.

@chrismclarke chrismclarke merged commit a55845a into main Jun 18, 2024
4 checks passed
@chrismclarke chrismclarke deleted the ft-forecast-pages branch June 18, 2024 16:12
@chrismclarke chrismclarke removed the Tool: Manual Updates related to Manual tool label Jul 8, 2024
@chrismclarke chrismclarke removed the Tool: Resources Updates related to Resources tool label Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Tool: Climate Updates related to Climate tool
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

feat(climate-tool): forecasts page
2 participants