-
-
Notifications
You must be signed in to change notification settings - Fork 730
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: add project flags component #6070
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
import { formatDateYMD } from 'utils/formatDate'; | ||
import { ExecutiveSummarySchema } from 'openapi'; | ||
|
||
const getRandomColor = () => { |
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.
can you add a sample screenshot so we can take a look at the colors that got generated? In other words I'd like to see if the color choice is smart enough to avoid very similar colors
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.
* See `gen:api` script in package.json | ||
*/ | ||
|
||
export type ExecutiveSummarySchemaProjectFlagTrendsItem = { |
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.
I recommend running it again one more time to get health and time to production that got added a second ago :) We'll avoid another orval run in future
theme: Theme, | ||
flagTrends: ExecutiveSummarySchema['projectFlagTrends'] = [], | ||
) => { | ||
// Group flag trends by project |
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.
unnecessary comment. variable name is similar
return groups; | ||
}, {}); | ||
|
||
// Create a dataset for each project |
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.
unnecessary comment
const date = | ||
item?.chart?.data?.labels?.[item.dataIndex]; | ||
return date | ||
? formatDateYMD(date, locationSettings.locale) |
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.
there should be override with 'UTC'. I think in the weekly scale it makes sense to show UTC time as we did for metrics
@@ -0,0 +1,5 @@ | |||
import { lazy } from 'react'; | |||
|
|||
export const FlagsProjectChart = lazy( |
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.
do we do this for each chart? maybe we can lazy load all charts that show up together
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.
Some comments inside
04e90be
to
04ca10d
Compare
This PR adds project flags line chart component