-
-
Notifications
You must be signed in to change notification settings - Fork 230
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: MetricCard
prototype
#2942
Conversation
✅ Deploy Preview for oss-insights ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for design-insights ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
we are putting this on hold for a sec until we figure out devstats and the API endpoint |
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 MetricCard component needs Storybook stories. There are plenty of examples in the repo for graphs, e.g. https://github.com/open-sauced/app/blob/758fb0b7f928ff35e7d71a49e6e6012f2f8b3d02/components/Graphs/MostUsedLanguagesGraph/most-used-languages-graph.stories.tsx
This way we can see the component as it's currently not on any page.
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.
Looking at the Storybook, looks solid!! 👏🏼 👏🏼
Just wondering if the graphs are supposed to be centered vertically with the value or align with the bottom? cc: @isabensusan |
I get a 404 when I visit https://deploy-preview-2942--oss-insights.netlify.app/s/open-sauced/app. Do I need to be added to the feature flag? |
If we can make the chart as tall as the number is, and align it to the bottom, it'd be perfect. But I think @zeucapua mentioned this is a bit gnarly to workout, so if it becomes a big blocker, I'm good to merge as is |
I wonder if it's just a question of trimming the canvase height. |
In terms of chart alignment, the line chart is weird since it starts at 1, so there's a gap at the actual bottom, so it feels like it's hovering above the bar chart. I can play around with the canvas to match the height of the number. I'll ping if it's too big of a blocker. |
Going to punt on the chart height, the canvas height and actual graph height aren't the same, and I can't manually change the chart height. Might need to pivot to another chart library. Would love to pair on this at some point @nickytonline |
We can look at another library potentially, but also, we can have a relative container and potentially move the chart that we'd give an absolute position to move it so it looks good. Anyways, we can chat about it. |
Probably worth humanizing the numbers. The 4 digit number is pushing the boundaries of the chart. |
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.
We can pair next week on the graph alignment. 🚢
## [2.13.0-beta.4](v2.13.0-beta.3...v2.13.0-beta.4) (2024-03-28) ### 🍕 Features * `MetricCard` prototype ([#2942](#2942)) ([1a57714](1a57714))
## [2.13.0](v2.12.1...v2.13.0) (2024-03-28) ### 🐛 Bug Fixes * enable edit and changing of name of contributor insight ([#3055](#3055)) ([970cd86](970cd86)) * redirect /hub/insights/new to login if not authenticated ([#2935](#2935)) ([55a73df](55a73df)) ### 🍕 Features * `MetricCard` prototype ([#2942](#2942)) ([1a57714](1a57714)) * prototype Stars Chart for Repo pages ([#3040](#3040)) ([cc74f8d](cc74f8d)) * **insights and lists:** unify UX for insights and lists ([#2582](#2582)) ([cbcfd20](cbcfd20))
Description
Implement
MetricCard
component that takes in stats returned by theuseFetchMetricStats
hook and the stat type (eg. 'stars', 'forks'). It parses the data and renders the graph, using Apache Echarts (TBD if to be replaced with a new chart library). Other parts of the card (number, percentage vs last period, button) are not implemented in this PR.What type of PR is this? (check all applicable)
Related Tickets & Documents
Closes #2870
Mobile & Desktop Screenshots/Recordings
Steps to QA
Added to documentation?
[optional] Are there any post-deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?