-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
add dynamic dates for webhookGraphDataUsage #7720
Changes from 1 commit
c7aa270
d5bf442
64757f7
bd25f57
f12dd40
a5d3d69
e3ef11e
e4eb06c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
import styled from '@emotion/styled'; | ||
import { format } from 'date-fns'; | ||
import { ReactElement } from 'react'; | ||
const StyledTooltipContainer = styled.div` | ||
align-items: center; | ||
border-radius: ${({ theme }) => theme.border.radius.md}; | ||
border: 1px solid ${({ theme }) => theme.border.color.medium}; | ||
display: flex; | ||
width: 128px; | ||
flex-direction: column; | ||
justify-content: center; | ||
background: ${({ theme }) => theme.background.transparent.secondary}; | ||
box-shadow: ${({ theme }) => theme.boxShadow.light}; | ||
backdrop-filter: ${({ theme }) => theme.blur.medium}; | ||
`; | ||
|
||
const StyledTooltipDateContainer = styled.div` | ||
align-items: flex-start; | ||
align-self: stretch; | ||
display: flex; | ||
justify-content: center; | ||
font-weight: ${({ theme }) => theme.font.weight.medium}; | ||
font-family: ${({ theme }) => theme.font.family}; | ||
gap: ${({ theme }) => theme.spacing(2)}; | ||
color: ${({ theme }) => theme.font.color.secondary}; | ||
padding: ${({ theme }) => theme.spacing(2)}; | ||
`; | ||
|
||
const StyledTooltipDataRow = styled.div` | ||
align-items: flex-start; | ||
align-self: stretch; | ||
display: flex; | ||
justify-content: space-between; | ||
color: ${({ theme }) => theme.font.color.tertiary}; | ||
padding: ${({ theme }) => theme.spacing(2)}; | ||
`; | ||
|
||
const StyledLine = styled.div` | ||
background-color: ${({ theme }) => theme.border.color.medium}; | ||
height: 1px; | ||
width: 100%; | ||
`; | ||
const StyledColorPoint = styled.div<{ color: string }>` | ||
background-color: ${({ color }) => color}; | ||
border-radius: 50%; | ||
height: 8px; | ||
width: 8px; | ||
display: inline-block; | ||
`; | ||
const StyledDataDefinition = styled.div` | ||
display: flex; | ||
align-items: center; | ||
gap: ${({ theme }) => theme.spacing(2)}; | ||
`; | ||
const StyledSpan = styled.span` | ||
color: ${({ theme }) => theme.font.color.primary}; | ||
`; | ||
|
||
export const SettingsDevelopersWebhookTooltip = ({ | ||
point, | ||
}: any): ReactElement => { | ||
const newDate = new Date(point.data.x); | ||
return ( | ||
<StyledTooltipContainer> | ||
<StyledTooltipDateContainer> | ||
{format(newDate, 'MMM d · HH:mm')} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: Use a consistent date format across the application. Consider using a constant for the date format string There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with Greptile again, maybe rely on existing formatting options? or add a new helper similar to formatToHumanReadableTime, etc. |
||
</StyledTooltipDateContainer> | ||
<StyledLine /> | ||
<StyledTooltipDataRow> | ||
<StyledDataDefinition> | ||
<StyledColorPoint color={point.serieColor} /> | ||
{String(point.serieId)} | ||
FelixMalfait marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</StyledDataDefinition> | ||
<StyledSpan>{String(point.data.y)}</StyledSpan> | ||
FelixMalfait marked this conversation as resolved.
Show resolved
Hide resolved
|
||
</StyledTooltipDataRow> | ||
</StyledTooltipContainer> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,14 @@ | ||
import { SettingsDevelopersWebhookTooltip } from '@/settings/developers/webhook/components/SettingsDevelopersliceTooltip'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. syntax: SettingsDevelopersliceTooltip is misspelled There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with greptile, the filename should match the component |
||
import { webhookGraphDataState } from '@/settings/developers/webhook/states/webhookGraphDataState'; | ||
import { fetchGraphData } from '@/settings/developers/webhook/utils/fetchGraphData'; | ||
import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; | ||
import { Select } from '@/ui/input/components/Select'; | ||
import { useTheme } from '@emotion/react'; | ||
import styled from '@emotion/styled'; | ||
import { ResponsiveLine } from '@nivo/line'; | ||
import { Section } from '@react-email/components'; | ||
import { useRecoilValue } from 'recoil'; | ||
import { useState } from 'react'; | ||
import { useRecoilValue, useSetRecoilState } from 'recoil'; | ||
import { H2Title } from 'twenty-ui'; | ||
|
||
export type NivoLineInput = { | ||
|
@@ -14,40 +20,160 @@ export type NivoLineInput = { | |
}>; | ||
}; | ||
const StyledGraphContainer = styled.div` | ||
height: 200px; | ||
width: 100%; | ||
background-color: ${({ theme }) => theme.background.secondary}; | ||
border: 1px solid ${({ theme }) => theme.border.color.medium}; | ||
border-radius: ${({ theme }) => theme.border.radius.md}; | ||
height: 199px; | ||
|
||
padding: ${({ theme }) => theme.spacing(4, 2, 2, 2)}; | ||
width: 496px; | ||
`; | ||
const StyledTitleContainer = styled.div` | ||
align-items: flex-start; | ||
display: flex; | ||
justify-content: space-between; | ||
`; | ||
export const SettingsDeveloppersWebhookUsageGraph = () => { | ||
|
||
type SettingsDevelopersWebhookUsageGraphProps = { | ||
webhookId: string; | ||
}; | ||
|
||
export const SettingsDevelopersWebhookUsageGraph = ({ | ||
webhookId, | ||
}: SettingsDevelopersWebhookUsageGraphProps) => { | ||
const webhookGraphData = useRecoilValue(webhookGraphDataState); | ||
const setWebhookGraphData = useSetRecoilState(webhookGraphDataState); | ||
const theme = useTheme(); | ||
|
||
const [wGraphOptions, setwGraphOptions] = useState< | ||
'7D' | '1D' | '12H' | '4H' | ||
>('7D'); | ||
const { enqueueSnackBar } = useSnackBar(); | ||
|
||
return ( | ||
<> | ||
{webhookGraphData.length ? ( | ||
<Section> | ||
<H2Title title="Statistics" /> | ||
<StyledTitleContainer> | ||
<H2Title | ||
title="Activity" | ||
description="See your webhook activity over time" | ||
/> | ||
<Select | ||
dropdownId="test-id-webhook-graph" | ||
value={wGraphOptions} | ||
options={[ | ||
{ value: '7D', label: 'This week' }, | ||
{ value: '1D', label: 'Today' }, | ||
{ value: '12H', label: 'Last 12 hours' }, | ||
{ value: '4H', label: 'Last 4 hours' }, | ||
]} | ||
onChange={(wGraphOptions) => { | ||
setwGraphOptions(wGraphOptions); | ||
fetchGraphData({ | ||
webhookId, | ||
enqueueSnackBar, | ||
webhookOptions: wGraphOptions, | ||
}).then((graphInput) => { | ||
setWebhookGraphData(graphInput); | ||
}); | ||
}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: consider moving this logic to a separate useEffect hook for better separation of concerns |
||
/> | ||
</StyledTitleContainer> | ||
|
||
<StyledGraphContainer> | ||
<ResponsiveLine | ||
data={webhookGraphData} | ||
curve={'monotoneX'} | ||
enableArea={true} | ||
colors={(d) => d.color} | ||
margin={{ top: 0, right: 0, bottom: 50, left: 60 }} | ||
theme={{ | ||
text: { | ||
fill: theme.font.color.light, | ||
fontSize: theme.font.size.sm, | ||
fontFamily: theme.font.family, | ||
}, | ||
axis: { | ||
domain: { | ||
line: { | ||
stroke: theme.border.color.light, | ||
}, | ||
}, | ||
ticks: { | ||
line: { | ||
stroke: theme.border.color.light, | ||
}, | ||
}, | ||
}, | ||
grid: { | ||
line: { | ||
stroke: theme.border.color.light, | ||
}, | ||
}, | ||
|
||
crosshair: { | ||
line: { | ||
stroke: theme.font.color.light, | ||
strokeDasharray: '2 2', | ||
}, | ||
}, | ||
}} | ||
margin={{ top: 20, right: 0, bottom: 30, left: 30 }} | ||
xFormat="time:%Y-%m-%d %H:%M%" | ||
xScale={{ | ||
type: 'time', | ||
useUTC: false, | ||
format: '%Y-%m-%d %H:%M:%S', | ||
precision: 'hour', | ||
}} | ||
defs={[ | ||
{ | ||
colors: [ | ||
{ | ||
color: 'inherit', | ||
offset: 0, | ||
}, | ||
{ | ||
color: 'inherit', | ||
offset: 100, | ||
opacity: 0, | ||
}, | ||
], | ||
id: 'gradientGraph', | ||
type: 'linearGradient', | ||
}, | ||
]} | ||
fill={[ | ||
{ | ||
id: 'gradientGraph', | ||
match: '*', | ||
}, | ||
]} | ||
yScale={{ | ||
type: 'linear', | ||
}} | ||
axisBottom={{ | ||
tickValues: 'every day', | ||
format: '%b %d', | ||
format: '%b %d, %I:%M %p', | ||
tickValues: 2, | ||
tickPadding: 5, | ||
tickSize: 6, | ||
}} | ||
Comment on lines
151
to
+156
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. logic: tickValues: 2 might not provide enough detail for larger time ranges |
||
axisLeft={{ | ||
tickPadding: 5, | ||
tickSize: 6, | ||
tickValues: 4, | ||
}} | ||
enableTouchCrosshair={true} | ||
enableGridY={false} | ||
enableGridX={false} | ||
lineWidth={1} | ||
gridYValues={4} | ||
enablePoints={false} | ||
isInteractive={true} | ||
useMesh={true} | ||
enableSlices={false} | ||
enableCrosshair={false} | ||
Comment on lines
+168
to
+169
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: enableSlices: false might limit user interaction with the graph |
||
tooltip={({ point }) => ( | ||
<SettingsDevelopersWebhookTooltip point={point} /> | ||
)} | ||
/> | ||
</StyledGraphContainer> | ||
</Section> | ||
|
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.
style: Consider defining a proper type for the 'point' prop instead of using 'any'
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 agree with greptile, you should shouldn't use any here :)