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

Feature/f 158 linechart component additional functionalities for forecast #116

Conversation

plaume8
Copy link
Collaborator

@plaume8 plaume8 commented Dec 8, 2024

F-158 Linear

I have now added some functionalities to the LineChart that make it clean and easy to distinguish between forecast and non-forecast data.
On the /elements page, I’ve added three proposals for how a chart, including a forecast line, could look like.

However, I don’t think it makes sense to explicitly add extra parameters to the LineChart component to handle forecast data.
The components using the LineChart should be responsible for correctly setting the LineChart parameters. For example, the FSC component would be responsible for fetching the data and correctly setting the LineChart component's parameters (eg determining which line is dashed and which is not).

@plaume8 plaume8 self-assigned this Dec 8, 2024
Copy link

netlify bot commented Dec 8, 2024

Deploy Preview for wfp-hungermap ready!

Name Link
🔨 Latest commit 95dca63
🔍 Latest deploy log https://app.netlify.com/sites/wfp-hungermap/deploys/675753c22a27590008504e46
😎 Deploy Preview https://deploy-preview-116--wfp-hungermap.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@bohdangarchu bohdangarchu left a comment

Choose a reason for hiding this comment

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

I still think we need a consistent way of showing forecast data that is handled by the linechart component. Otherwise we might have a situation where every component is repeating the same code, or every component has different way to show prediction. My suggestion would be to add a flag or type to lines parameter indicating that a line is a forecast or not. And LineChart would apply the styles based on that. By the way I like the styling you applied, especially this one
image

@plaume8
Copy link
Collaborator Author

plaume8 commented Dec 9, 2024

@bohdangarchu @marinovl7 - I added the additional params predictionVerticalLineX and lines.prediction such that the prediction styling is automatically applied

note: the LineChartOperations is becoming quite big and complex -> i will refactor it a bit in F-165

@plaume8 plaume8 force-pushed the feature/f-158-linechart-component-additional-functionalities-for-forecast branch from 038e4b4 to 99bb99c Compare December 9, 2024 20:28
@plaume8 plaume8 requested a review from bohdangarchu December 9, 2024 20:29
Copy link
Collaborator

@bohdangarchu bohdangarchu left a comment

Choose a reason for hiding this comment

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

good job!

Copy link
Collaborator

@marinovl7 marinovl7 left a comment

Choose a reason for hiding this comment

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

LGTM!

@marinovl7 marinovl7 merged commit 3a0272d into main Dec 10, 2024
5 checks passed
@marinovl7 marinovl7 deleted the feature/f-158-linechart-component-additional-functionalities-for-forecast branch December 10, 2024 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants