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

add dynamic dates for webhookGraphDataUsage #7720

Merged
merged 8 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 => {
Copy link
Contributor

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'

Copy link
Member

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 :)

const newDate = new Date(point.data.x);
return (
<StyledTooltipContainer>
<StyledTooltipDateContainer>
{format(newDate, 'MMM d · HH:mm')}
Copy link
Contributor

Choose a reason for hiding this comment

The 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

Copy link
Member

Choose a reason for hiding this comment

The 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';
Copy link
Contributor

Choose a reason for hiding this comment

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

syntax: SettingsDevelopersliceTooltip is misspelled

Copy link
Member

Choose a reason for hiding this comment

The 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 = {
Expand All @@ -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);
});
}}
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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
Copy link
Contributor

Choose a reason for hiding this comment

The 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>
Expand Down
Loading
Loading