Refactor: Replace inline style objects with makeStyles classes
Moved all static layout properties (display, gap, margin, padding, colour) from inline style props to makeStyles classes in: - MapBansTable.tsx: Pagination row flexbox layout - JailDetailPage.tsx: Link styling for textDecoration - HistoryPage.tsx: Summary text styling - IpDetailView.tsx: Loading container and text formatting Kept inline styles only for genuinely dynamic values: - WorldMap.tsx: Tooltip position (follows mouse) - TopCountriesPieChart.tsx: Legend color (from recharts data) - TopCountriesBarChart.tsx: Chart height (derives from data length) This change improves performance by leveraging Griffel's atomic CSS cache and ensures consistency with the established Fluent UI pattern. Updated Docs/Web-Development.md with explicit rule: inline styles only for runtime-dynamic values, all static properties go in makeStyles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1,23 +1,3 @@
|
|||||||
### T-19 · Move `DashboardFilterProvider` — page-scoped provider in wrong directory
|
|
||||||
|
|
||||||
**Where found:** `frontend/src/providers/DashboardFilterProvider.tsx` — instantiated only inside `DashboardPage.tsx`
|
|
||||||
|
|
||||||
**Why this is needed:** The `providers/` directory implies app-wide providers (alongside `AuthProvider`, `ThemeProvider`, `TimezoneProvider`). `DashboardFilterProvider` wraps only `DashboardPageContent` and is not used anywhere else. Its placement implies reuse that doesn't exist, misleading future contributors about its scope.
|
|
||||||
|
|
||||||
**Goal:** Co-located with its only consumer.
|
|
||||||
|
|
||||||
**What to do:**
|
|
||||||
1. Move `DashboardFilterProvider.tsx` to `frontend/src/pages/` (alongside `DashboardPage.tsx`) or to `frontend/src/pages/dashboard/` if the page is split into a subdirectory.
|
|
||||||
2. Update imports in `DashboardPage.tsx` and any tests.
|
|
||||||
|
|
||||||
**Possible traps and issues:** Only `DashboardPage.tsx` imports it — confirm with grep before moving.
|
|
||||||
|
|
||||||
**Docs changes needed:** `Docs/Web-Development.md` — document what belongs in `providers/` (app-wide) vs co-located.
|
|
||||||
|
|
||||||
**Doc references:** `Docs/Web-Development.md`
|
|
||||||
|
|
||||||
---
|
|
||||||
|
|
||||||
### T-20 · Replace inline `style={{}}` objects with `makeStyles` classes
|
### T-20 · Replace inline `style={{}}` objects with `makeStyles` classes
|
||||||
|
|
||||||
**Where found:** `frontend/src/pages/map/MapBansTable.tsx` (multiple), `pages/JailDetailPage.tsx`, `pages/HistoryPage.tsx`, `pages/history/IpDetailView.tsx`, `components/WorldMap.tsx`, `components/TopCountriesPieChart.tsx`, `components/TopCountriesBarChart.tsx`
|
**Where found:** `frontend/src/pages/map/MapBansTable.tsx` (multiple), `pages/JailDetailPage.tsx`, `pages/HistoryPage.tsx`, `pages/history/IpDetailView.tsx`, `components/WorldMap.tsx`, `components/TopCountriesPieChart.tsx`, `components/TopCountriesBarChart.tsx`
|
||||||
|
|||||||
@@ -257,6 +257,7 @@ export const darkTheme: Theme = createDarkTheme(brandColors);
|
|||||||
- Co-locate styles in the same file as the component they belong to, defined above the component function.
|
- Co-locate styles in the same file as the component they belong to, defined above the component function.
|
||||||
- Use `mergeClasses` when combining multiple style sets conditionally.
|
- Use `mergeClasses` when combining multiple style sets conditionally.
|
||||||
- Reference Fluent **design tokens** (`tokens.colorBrandBackground`, `tokens.fontSizeBase300`, etc.) instead of hard-coded values — this ensures consistency and automatic theme support.
|
- Reference Fluent **design tokens** (`tokens.colorBrandBackground`, `tokens.fontSizeBase300`, etc.) instead of hard-coded values — this ensures consistency and automatic theme support.
|
||||||
|
- **Inline styles are only allowed for genuinely dynamic values** (e.g., tooltip position calculated from mouse position, or height derived from data count). All static layout properties (`display`, `gap`, `margin`, `padding`, colour) must go in `makeStyles`.
|
||||||
|
|
||||||
```tsx
|
```tsx
|
||||||
import { makeStyles, tokens, mergeClasses } from "@fluentui/react-components";
|
import { makeStyles, tokens, mergeClasses } from "@fluentui/react-components";
|
||||||
|
|||||||
@@ -116,6 +116,9 @@ const useStyles = makeStyles({
|
|||||||
fontFamily: "Consolas, 'Courier New', monospace",
|
fontFamily: "Consolas, 'Courier New', monospace",
|
||||||
fontSize: "0.85rem",
|
fontSize: "0.85rem",
|
||||||
},
|
},
|
||||||
|
summaryText: {
|
||||||
|
color: tokens.colorNeutralForeground3,
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
@@ -292,7 +295,7 @@ export function HistoryPage(): React.JSX.Element {
|
|||||||
{/* Summary */}
|
{/* Summary */}
|
||||||
{/* ---------------------------------------------------------------- */}
|
{/* ---------------------------------------------------------------- */}
|
||||||
{!loading && !error && (
|
{!loading && !error && (
|
||||||
<Text size={300} style={{ color: tokens.colorNeutralForeground3 }}>
|
<Text size={300} className={styles.summaryText}>
|
||||||
{String(total)} record{total !== 1 ? "s" : ""} found ·
|
{String(total)} record{total !== 1 ? "s" : ""} found ·
|
||||||
Page {String(currentPage)} of {String(totalPages)} ·
|
Page {String(currentPage)} of {String(totalPages)} ·
|
||||||
Rows highlighted in yellow have {String(HIGH_BAN_THRESHOLD)}+ repeat bans
|
Rows highlighted in yellow have {String(HIGH_BAN_THRESHOLD)}+ repeat bans
|
||||||
|
|||||||
@@ -5,7 +5,7 @@
|
|||||||
* identified by the `:name` route parameter.
|
* identified by the `:name` route parameter.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { Button, MessageBar, MessageBarBody, Spinner, Text } from "@fluentui/react-components";
|
import { Button, MessageBar, MessageBarBody, Spinner, Text, makeStyles } from "@fluentui/react-components";
|
||||||
import { ArrowLeftRegular } from "@fluentui/react-icons";
|
import { ArrowLeftRegular } from "@fluentui/react-icons";
|
||||||
import { Link, useParams } from "react-router-dom";
|
import { Link, useParams } from "react-router-dom";
|
||||||
import { useJailData } from "../hooks/useJailData";
|
import { useJailData } from "../hooks/useJailData";
|
||||||
@@ -18,7 +18,14 @@ import { BantimeEscalationSection } from "../components/jail/BantimeEscalationSe
|
|||||||
import { IgnoreListSection } from "../components/jail/IgnoreListSection";
|
import { IgnoreListSection } from "../components/jail/IgnoreListSection";
|
||||||
import { useJailDetailPageStyles } from "../components/jail/jailDetailPageStyles";
|
import { useJailDetailPageStyles } from "../components/jail/jailDetailPageStyles";
|
||||||
|
|
||||||
|
const useLinkStyles = makeStyles({
|
||||||
|
link: {
|
||||||
|
textDecoration: "none",
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
export function JailDetailPage(): React.JSX.Element {
|
export function JailDetailPage(): React.JSX.Element {
|
||||||
|
const linkStyles = useLinkStyles();
|
||||||
const styles = useJailDetailPageStyles();
|
const styles = useJailDetailPageStyles();
|
||||||
const { name = "" } = useParams<{ name: string }>();
|
const { name = "" } = useParams<{ name: string }>();
|
||||||
const { jail, ignoreList, ignoreSelf, loading, error, refresh } = useJailData(name);
|
const { jail, ignoreList, ignoreSelf, loading, error, refresh } = useJailData(name);
|
||||||
@@ -50,7 +57,7 @@ export function JailDetailPage(): React.JSX.Element {
|
|||||||
if (error) {
|
if (error) {
|
||||||
return (
|
return (
|
||||||
<div className={styles.root}>
|
<div className={styles.root}>
|
||||||
<Link to="/jails" style={{ textDecoration: "none" }}>
|
<Link to="/jails" className={linkStyles.link}>
|
||||||
<Button appearance="subtle" icon={<ArrowLeftRegular />}>
|
<Button appearance="subtle" icon={<ArrowLeftRegular />}>
|
||||||
Back to Jails
|
Back to Jails
|
||||||
</Button>
|
</Button>
|
||||||
@@ -67,7 +74,7 @@ export function JailDetailPage(): React.JSX.Element {
|
|||||||
return (
|
return (
|
||||||
<div className={styles.root}>
|
<div className={styles.root}>
|
||||||
<div className={styles.breadcrumb}>
|
<div className={styles.breadcrumb}>
|
||||||
<Link to="/jails" style={{ textDecoration: "none" }}>
|
<Link to="/jails" className={linkStyles.link}>
|
||||||
<Button appearance="subtle" size="small" icon={<ArrowLeftRegular />}>
|
<Button appearance="subtle" size="small" icon={<ArrowLeftRegular />}>
|
||||||
Jails
|
Jails
|
||||||
</Button>
|
</Button>
|
||||||
|
|||||||
@@ -67,6 +67,24 @@ const useStyles = makeStyles({
|
|||||||
borderRadius: tokens.borderRadiusMedium,
|
borderRadius: tokens.borderRadiusMedium,
|
||||||
border: `1px solid ${tokens.colorNeutralStroke1}`,
|
border: `1px solid ${tokens.colorNeutralStroke1}`,
|
||||||
},
|
},
|
||||||
|
loadingContainer: {
|
||||||
|
display: "flex",
|
||||||
|
justifyContent: "center",
|
||||||
|
padding: tokens.spacingVerticalXL,
|
||||||
|
},
|
||||||
|
headerControls: {
|
||||||
|
display: "flex",
|
||||||
|
alignItems: "center",
|
||||||
|
gap: tokens.spacingHorizontalM,
|
||||||
|
},
|
||||||
|
dimmedText: {
|
||||||
|
color: tokens.colorNeutralForeground3,
|
||||||
|
},
|
||||||
|
monoPreformatted: {
|
||||||
|
fontFamily: "Consolas, 'Courier New', monospace",
|
||||||
|
whiteSpace: "pre-wrap",
|
||||||
|
wordBreak: "break-all",
|
||||||
|
},
|
||||||
});
|
});
|
||||||
|
|
||||||
export function IpDetailView({ ip, onBack }: IpDetailViewProps): React.JSX.Element {
|
export function IpDetailView({ ip, onBack }: IpDetailViewProps): React.JSX.Element {
|
||||||
@@ -76,7 +94,7 @@ export function IpDetailView({ ip, onBack }: IpDetailViewProps): React.JSX.Eleme
|
|||||||
|
|
||||||
if (loading) {
|
if (loading) {
|
||||||
return (
|
return (
|
||||||
<div style={{ display: "flex", justifyContent: "center", padding: tokens.spacingVerticalXL }}>
|
<div className={styles.loadingContainer}>
|
||||||
<Spinner label={`Loading history for ${ip}…`} />
|
<Spinner label={`Loading history for ${ip}…`} />
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
@@ -101,7 +119,7 @@ export function IpDetailView({ ip, onBack }: IpDetailViewProps): React.JSX.Eleme
|
|||||||
return (
|
return (
|
||||||
<div className={styles.root}>
|
<div className={styles.root}>
|
||||||
<div className={styles.header}>
|
<div className={styles.header}>
|
||||||
<div style={{ display: "flex", alignItems: "center", gap: tokens.spacingHorizontalM }}>
|
<div className={styles.headerControls}>
|
||||||
<Button icon={<ArrowLeftRegular />} appearance="subtle" onClick={onBack}>
|
<Button icon={<ArrowLeftRegular />} appearance="subtle" onClick={onBack}>
|
||||||
Back to list
|
Back to list
|
||||||
</Button>
|
</Button>
|
||||||
@@ -176,17 +194,13 @@ export function IpDetailView({ ip, onBack }: IpDetailViewProps): React.JSX.Eleme
|
|||||||
<TableCell>
|
<TableCell>
|
||||||
<TableCellLayout>
|
<TableCellLayout>
|
||||||
{event.matches.length === 0 ? (
|
{event.matches.length === 0 ? (
|
||||||
<Text size={200} style={{ color: tokens.colorNeutralForeground3 }}>
|
<Text size={200} className={styles.dimmedText}>
|
||||||
—
|
—
|
||||||
</Text>
|
</Text>
|
||||||
) : (
|
) : (
|
||||||
<Text
|
<Text
|
||||||
size={100}
|
size={100}
|
||||||
style={{
|
className={styles.monoPreformatted}
|
||||||
fontFamily: "Consolas, 'Courier New', monospace",
|
|
||||||
whiteSpace: "pre-wrap",
|
|
||||||
wordBreak: "break-all",
|
|
||||||
}}
|
|
||||||
>
|
>
|
||||||
{event.matches.join("\n")}
|
{event.matches.join("\n")}
|
||||||
</Text>
|
</Text>
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import {
|
|||||||
TableRow,
|
TableRow,
|
||||||
Text,
|
Text,
|
||||||
Tooltip,
|
Tooltip,
|
||||||
|
makeStyles,
|
||||||
tokens,
|
tokens,
|
||||||
} from "@fluentui/react-components";
|
} from "@fluentui/react-components";
|
||||||
import { ChevronLeftRegular, ChevronRightRegular } from "@fluentui/react-icons";
|
import { ChevronLeftRegular, ChevronRightRegular } from "@fluentui/react-icons";
|
||||||
@@ -27,6 +28,33 @@ interface MapBansTableProps {
|
|||||||
onPageSizeChange: (pageSize: number) => void;
|
onPageSizeChange: (pageSize: number) => void;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const useStyles = makeStyles({
|
||||||
|
paginationRow: {
|
||||||
|
display: "flex",
|
||||||
|
alignItems: "center",
|
||||||
|
justifyContent: "space-between",
|
||||||
|
gap: tokens.spacingHorizontalS,
|
||||||
|
marginTop: tokens.spacingVerticalS,
|
||||||
|
},
|
||||||
|
paginationLeft: {
|
||||||
|
display: "flex",
|
||||||
|
alignItems: "center",
|
||||||
|
gap: tokens.spacingHorizontalS,
|
||||||
|
},
|
||||||
|
pageSize: {
|
||||||
|
display: "flex",
|
||||||
|
alignItems: "center",
|
||||||
|
gap: tokens.spacingHorizontalS,
|
||||||
|
},
|
||||||
|
paginationButtons: {
|
||||||
|
display: "flex",
|
||||||
|
gap: tokens.spacingHorizontalXS,
|
||||||
|
},
|
||||||
|
dimmedText: {
|
||||||
|
color: tokens.colorNeutralForeground3,
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
export function MapBansTable({
|
export function MapBansTable({
|
||||||
pageBans,
|
pageBans,
|
||||||
visibleCount,
|
visibleCount,
|
||||||
@@ -38,6 +66,7 @@ export function MapBansTable({
|
|||||||
onPageChange,
|
onPageChange,
|
||||||
onPageSizeChange,
|
onPageSizeChange,
|
||||||
}: MapBansTableProps): React.JSX.Element {
|
}: MapBansTableProps): React.JSX.Element {
|
||||||
|
const styles = useStyles();
|
||||||
return (
|
return (
|
||||||
<>
|
<>
|
||||||
<Table size="small" aria-label="Bans list">
|
<Table size="small" aria-label="Bans list">
|
||||||
@@ -56,7 +85,7 @@ export function MapBansTable({
|
|||||||
<TableRow>
|
<TableRow>
|
||||||
<TableCell colSpan={6}>
|
<TableCell colSpan={6}>
|
||||||
<TableCellLayout>
|
<TableCellLayout>
|
||||||
<Text size={200} style={{ color: tokens.colorNeutralForeground3 }}>
|
<Text size={200} className={styles.dimmedText}>
|
||||||
No bans found.
|
No bans found.
|
||||||
</Text>
|
</Text>
|
||||||
</TableCellLayout>
|
</TableCellLayout>
|
||||||
@@ -83,7 +112,7 @@ export function MapBansTable({
|
|||||||
content="Country could not be resolved — will retry automatically."
|
content="Country could not be resolved — will retry automatically."
|
||||||
relationship="description"
|
relationship="description"
|
||||||
>
|
>
|
||||||
<Text style={{ color: tokens.colorNeutralForeground3 }}>—</Text>
|
<Text className={styles.dimmedText}>—</Text>
|
||||||
</Tooltip>
|
</Tooltip>
|
||||||
)}
|
)}
|
||||||
</TableCellLayout>
|
</TableCellLayout>
|
||||||
@@ -108,14 +137,14 @@ export function MapBansTable({
|
|||||||
</Table>
|
</Table>
|
||||||
|
|
||||||
{visibleCount > 0 && (
|
{visibleCount > 0 && (
|
||||||
<div style={{ display: "flex", alignItems: "center", justifyContent: "space-between", gap: tokens.spacingHorizontalS, marginTop: tokens.spacingVerticalS }}>
|
<div className={styles.paginationRow}>
|
||||||
<div style={{ display: "flex", alignItems: "center", gap: tokens.spacingHorizontalS }}>
|
<div className={styles.paginationLeft}>
|
||||||
<Text size={200} style={{ color: tokens.colorNeutralForeground3 }}>
|
<Text size={200} className={styles.dimmedText}>
|
||||||
Showing {pageBans.length} of {visibleCount} filtered ban{visibleCount !== 1 ? "s" : ""}
|
Showing {pageBans.length} of {visibleCount} filtered ban{visibleCount !== 1 ? "s" : ""}
|
||||||
{" · "}Page {page} of {totalPages}
|
{" · "}Page {page} of {totalPages}
|
||||||
</Text>
|
</Text>
|
||||||
<div style={{ display: "flex", alignItems: "center", gap: tokens.spacingHorizontalS }}>
|
<div className={styles.pageSize}>
|
||||||
<Text size={200} style={{ color: tokens.colorNeutralForeground3 }}>
|
<Text size={200} className={styles.dimmedText}>
|
||||||
Page size
|
Page size
|
||||||
</Text>
|
</Text>
|
||||||
<select
|
<select
|
||||||
@@ -134,7 +163,7 @@ export function MapBansTable({
|
|||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
<div style={{ display: "flex", gap: tokens.spacingHorizontalXS }}>
|
<div className={styles.paginationButtons}>
|
||||||
<Button
|
<Button
|
||||||
icon={<ChevronLeftRegular />}
|
icon={<ChevronLeftRegular />}
|
||||||
appearance="subtle"
|
appearance="subtle"
|
||||||
|
|||||||
Reference in New Issue
Block a user