-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
chore: update comments #86
base: dev
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
@@ -1,5 +1,19 @@ | ||
import React from 'react'; | ||
import { EventCardProps } from '../types'; // Ensure this matches the updated structure | ||
import { EventCardProps } from '../types'; | ||
/** | ||
* EventCard component displays information about an event and provides a button to view more details. | ||
* | ||
* @component | ||
* @param {Object} props - The props object. | ||
* @param {Object} props.event - The event object containing details about the event. | ||
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. I think we can use our own types here (TGEvent instead of Object as a type is more descriptive) |
||
* @param {string} props.event.name - The name of the event. | ||
* @param {string} props.event.time - The timestamp of the event. | ||
* @param {string} props.event.username - The username of the person associated with the event. | ||
* @param {Function} props.onViewDetails - Callback function to handle viewing event details. | ||
* | ||
* @returns {JSX.Element} The rendered EventCard component. | ||
*/ | ||
|
||
|
||
const EventCard: React.FC<EventCardProps> = ({ event, onViewDetails }) => { | ||
// Ensure event.time is a valid Date object | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,16 +1,45 @@ | ||
import { useState, useEffect } from 'react'; | ||
import AccessPerIpChart from './charts/AccessPerIp'; | ||
import IpAccessOverTimeChart from './charts/IpAccessOverTime'; | ||
import { CountedEvent, IPLocation, IpAccessCombinedProps } from '../types'; // Import the interface from types.ts | ||
import { CountedEvent, IPLocation, IpAccessCombinedProps } from '../types'; | ||
|
||
/** | ||
* IpAccessCombined component fetches and displays IP location counts and details for a selected IP. | ||
* | ||
* @component | ||
* @param {Object} props - The component props. | ||
* @param {string} props.currentIp - The currently selected IP address. | ||
* @param {Function} props.setCurrentIp - Function to update the currently selected IP address. | ||
* @returns {JSX.Element} The rendered component. | ||
* | ||
* @typedef {Object} IPLocation | ||
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. Oooo, Okay maybe this is how we should be doing it then. Didn't know about this formatting. |
||
* @property {string} city - The city of the IP location. | ||
* @property {string} region - The region of the IP location. | ||
* @property {string} country - The country of the IP location. | ||
* @property {string} source_ip - The source IP address. | ||
* | ||
* @typedef {Object} CountedEvent | ||
* @property {number} count - The count of events for the IP. | ||
* | ||
* @typedef {Object} IpAccessCombinedProps | ||
* @property {string} currentIp - The currently selected IP address. | ||
* @property {Function} setCurrentIp - Function to update the currently selected IP address. | ||
* | ||
* @typedef {IPLocation & CountedEvent} IPLocationCountedEvent | ||
* | ||
* @example | ||
* <IpAccessCombined currentIp="192.168.1.1" setCurrentIp={setIp} /> | ||
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. also sweet |
||
*/ | ||
export default function IpAccessCombined({ | ||
currentIp, | ||
setCurrentIp, | ||
}: IpAccessCombinedProps): JSX.Element { | ||
// State to hold the IP location counts | ||
const [ipLocCounts, setIpLocCounts] = useState<(IPLocation & CountedEvent)[]>( | ||
[] | ||
); | ||
|
||
// Fetch IP location counts when the component mounts | ||
useEffect(() => { | ||
fetch('/events?countOn=source_ip&includeLocation=true') | ||
.then((response) => { | ||
|
@@ -25,26 +54,29 @@ export default function IpAccessCombined({ | |
); | ||
}, []); | ||
|
||
// Find the location details of the current IP | ||
const currentIpLoc = ipLocCounts.find( | ||
({ source_ip }) => source_ip === currentIp | ||
); | ||
|
||
return ( | ||
<> | ||
<div> | ||
{/* Render the AccessPerIpChart component */} | ||
<AccessPerIpChart | ||
currentIp={currentIp} | ||
setCurrentIp={setCurrentIp} | ||
ipLocCounts={ipLocCounts} | ||
/> | ||
{currentIp && ( | ||
<> | ||
{/* Display the location details of the current IP */} | ||
<p> | ||
<strong>Location: </strong> | ||
{currentIpLoc?.city}, {currentIpLoc?.region},{' '} | ||
{currentIpLoc?.country} | ||
</p> | ||
{/* Make sure the chart renders only when IP is selected */} | ||
{/* TODO: Render the IpAccessOverTimeChart component only when an IP is selected//possible display styling issue */} | ||
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. This is currently how it works, (see line 71) |
||
<IpAccessOverTimeChart currentIp={currentIp} /> | ||
</> | ||
)} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,34 @@ import { useState, useEffect } from 'react'; | |
import { Cell, Legend, Pie, PieChart } from 'recharts'; | ||
import { CountedEvent, IPLocation } from '../../types'; | ||
|
||
// Define colors for the pie chart slices | ||
const COLORS = ['#0088FE', '#00C49F', '#FFBB28', '#FF8042']; | ||
|
||
/** | ||
* Component to render a pie chart displaying access counts per IP location. | ||
* | ||
* @param {Object} props - The component props. | ||
* @param {string} [props.currentIp] - The currently selected IP address. | ||
* @param {React.Dispatch<React.SetStateAction<string | undefined>>} props.setCurrentIp - Function to set the currently selected IP address. | ||
* @param {Array<IPLocation & CountedEvent>} props.ipLocCounts - Array of IP location counts. | ||
* @returns {JSX.Element} The rendered pie chart component. | ||
* | ||
* @component | ||
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. This was before the @params in another jsdoc, does the order matter? |
||
* @example | ||
* const currentIp = "192.168.1.1"; | ||
* const setCurrentIp = (ip) => console.log(ip); | ||
* const ipLocCounts = [ | ||
* { source_ip: "192.168.1.1", count: 10, location: "Location A" }, | ||
* { source_ip: "192.168.1.2", count: 5, location: "Location B" } | ||
* ]; | ||
* return ( | ||
* <AccessPerIpChart | ||
* currentIp={currentIp} | ||
* setCurrentIp={setCurrentIp} | ||
* ipLocCounts={ipLocCounts} | ||
* /> | ||
* ); | ||
*/ | ||
export default function AccessPerIpChart({ | ||
currentIp, | ||
setCurrentIp, | ||
|
@@ -13,7 +39,7 @@ export default function AccessPerIpChart({ | |
setCurrentIp: React.Dispatch<React.SetStateAction<string | undefined>>; | ||
ipLocCounts: (IPLocation & CountedEvent)[]; | ||
}): JSX.Element { | ||
const [loading, setLoading] = useState(true); // Add loading state | ||
const [loading, setLoading] = useState(true); | ||
|
||
useEffect(() => { | ||
// Simulate loading delay for data | ||
|
@@ -24,6 +50,8 @@ export default function AccessPerIpChart({ | |
}, [ipLocCounts]); | ||
|
||
const RADIAN = Math.PI / 180; | ||
|
||
// Function to render custom labels inside the pie chart slices | ||
const renderCustomizedLabel = ({ | ||
cx, | ||
cy, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,7 @@ import { useEffect, useState } from 'react'; | |
import { Bar, BarChart, LabelList, XAxis, Cell } from 'recharts'; | ||
import { CountedEvent } from '../../types'; | ||
|
||
// Array of colors for the chart | ||
const COLORS = [ | ||
'#0088FE', | ||
'#00C49F', | ||
|
@@ -12,13 +13,46 @@ const COLORS = [ | |
'#FFCC99', | ||
]; | ||
|
||
/** | ||
* EventSourceChart component renders a bar chart of event sources. | ||
* | ||
* @component | ||
* @example | ||
* return ( | ||
* <EventSourceChart /> | ||
* ) | ||
* | ||
* @returns {JSX.Element} The rendered bar chart component. | ||
* | ||
* @remarks | ||
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. !! |
||
* This component fetches event data from the server and displays it in a bar chart. | ||
* It also allows users to click on a bar to select an event source, which is displayed below the chart. | ||
* | ||
* @typedef {Object} CountedEvent | ||
* @property {string} source - The source of the event. | ||
* @property {number} count - The count of events from this source. | ||
* | ||
* @state {CountedEvent[]} events - The fetched events data. | ||
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. !! |
||
* @state {boolean} loading - The loading status of the data fetch. | ||
* @state {string | null} selectedEventSource - The selected event source when a bar is clicked. | ||
* | ||
* @hook {useEffect} Fetches events data on component mount. | ||
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. !! |
||
* | ||
* @function handleClick | ||
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. !! |
||
* @description Handles click event on a bar to toggle the selected event source. | ||
* @param {string} source - The source of the event that was clicked. | ||
*/ | ||
export default function EventSourceChart() { | ||
// State to store the fetched events data | ||
const [events, setEvents] = useState<CountedEvent[]>([]); | ||
// State to manage loading status | ||
const [loading, setLoading] = useState(true); | ||
// State to manage the selected event source when a bar is clicked | ||
const [selectedEventSource, setSelectedEventSource] = useState<string | null>( | ||
null | ||
); // State for clicked event source | ||
); | ||
|
||
// Fetch events data on component mount | ||
useEffect(() => { | ||
setLoading(true); | ||
fetch('/events?countOn=source') | ||
|
@@ -35,8 +69,10 @@ export default function EventSourceChart() { | |
); | ||
}, []); | ||
|
||
// Display loading message while data is being fetched | ||
if (loading) return <p>Loading chart...</p>; | ||
|
||
// Handle click event on a bar to toggle the selected event source | ||
const handleClick = (source: string) => { | ||
setSelectedEventSource((prevSelected) => | ||
prevSelected === source ? null : source | ||
|
@@ -46,7 +82,7 @@ export default function EventSourceChart() { | |
return ( | ||
<div style={{ width: '100%', overflowX: 'auto' }}> | ||
<BarChart | ||
width={events.length * 120} | ||
width={events.length * 120} // Dynamic width based on the number of events | ||
height={300} | ||
data={events} | ||
layout="horizontal" | ||
|
@@ -64,7 +100,7 @@ export default function EventSourceChart() { | |
{events.map((entry, index) => ( | ||
<Cell | ||
key={`cell-${index}`} | ||
fill={COLORS[index % COLORS.length]} | ||
fill={COLORS[index % COLORS.length]} // Assign color from the COLORS array | ||
onClick={() => handleClick(entry.source)} // Attach the click handler to each Cell | ||
style={{ cursor: 'pointer' }} // Add a pointer cursor to indicate clickability | ||
/> | ||
|
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 think we want to specify the tiltle, children and darkmode as props properties,
The way this is currently formatted makes it seem like the component takes 4 input parameters: props, title, children, isDarkMode. (I think the props.____ might be confusing, and the limited description for props as well)
How does the VScode mouseover look for this? If it is similar to other react component's documentation, you can disregard this
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.
Using CardProps as the type of props instead of Object may help
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 looked into more JSDoc documentation, and using dot notation seems to be the standard way to do this, so I think the CardProps type is the only change needed here