Skip to content

Commit 636ba2f

Browse files
committed
refactor: theme typing, error UX, a11y, lint config, dev hygiene
Quality and hygiene follow-ups across the workspace: - ThemeProvider: introduce a ThemeMode union with a type-guarded sessionStorage read, keep render pure (storage is only written on an explicit user toggle), memoize toggleTheme, and derive outlined-input border/label colors for both palette modes; Dashboard tests updated for the new persistence semantics - eslint flat configs (components, app, extensions) tightened, notably enabling eslint-plugin-jsx-a11y; the intentional autoFocus in the drawer rename fields gains documented disable directives - README refreshed - lockfile synchronised with the final dependency set
1 parent 4186c11 commit 636ba2f

11 files changed

Lines changed: 144 additions & 82 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
- **Developing a DiracX-Web extension?** Go straight to the [:page_facing_up: Extension README](https://diracx.diracgrid.org/en/latest/developer/manage_extension/)
2020
- **Managing the repository?** Discover tips and tricks in the [:book: Ops Guide](https://diracx.diracgrid.org/en/latest/dev/setup_environment/)
2121

22-
- **Interested in contributing?** Read the [:star: Contributing Document](CONTRIBUTING.md)
22+
- **Interested in contributing?** Read the [:star: Contributing Document](docs/dev/how-to/contribute-to-web.md)
2323

2424

2525

package-lock.json

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/diracx-web-components/eslint.config.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import _import from "eslint-plugin-import";
55
import typescriptEslint from "@typescript-eslint/eslint-plugin";
66
import react from "eslint-plugin-react";
77
import reactHooks from "eslint-plugin-react-hooks";
8+
import jsxA11y from "eslint-plugin-jsx-a11y";
89
import tsParser from "@typescript-eslint/parser";
910
import js from "@eslint/js";
1011
import { FlatCompat } from "@eslint/eslintrc";
@@ -30,6 +31,7 @@ export default [
3031
"prettier",
3132
),
3233
),
34+
jsxA11y.flatConfigs.recommended,
3335
{
3436
plugins: {
3537
import: fixupPluginRules(_import),
@@ -79,6 +81,19 @@ export default [
7981
ignoreRestSiblings: true,
8082
},
8183
],
84+
"@typescript-eslint/no-restricted-imports": [
85+
"error",
86+
{
87+
paths: [
88+
{
89+
name: "@mui/icons-material",
90+
message:
91+
"Import icons individually: import Icon from '@mui/icons-material/Icon'",
92+
allowTypeImports: true,
93+
},
94+
],
95+
},
96+
],
8297
"no-restricted-properties": [
8398
"error",
8499
{

packages/diracx-web-components/src/components/DashboardLayout/DrawerItem.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,7 @@ export default function DrawerItem({
224224
// The rename field only appears in direct response to a user
225225
// action (context-menu "Rename"), so moving focus into it is
226226
// the expected behavior, not a focus steal.
227+
// eslint-disable-next-line jsx-a11y/no-autofocus
227228
autoFocus
228229
size="small"
229230
/>

packages/diracx-web-components/src/components/DashboardLayout/DrawerItemGroup.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ export default function DrawerItemGroup({
143143
// The rename field only appears in direct response to a user
144144
// action (context-menu "Rename"), so moving focus into it is
145145
// the expected behavior, not a focus steal.
146+
// eslint-disable-next-line jsx-a11y/no-autofocus
146147
autoFocus
147148
size="small"
148149
/>

packages/diracx-web-components/src/components/Login/LoginForm.tsx

Lines changed: 42 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,18 @@
33
import React, { useState, useEffect, use } from "react";
44

55
import {
6+
Alert,
67
Box,
78
Typography,
89
FormControl,
910
InputLabel,
1011
Select,
1112
MenuItem,
1213
Button,
13-
Stack,
1414
Autocomplete,
1515
TextField,
1616
SelectChangeEvent,
17+
Skeleton,
1718
} from "@mui/material";
1819

1920
import { useOidc } from "@axa-fr/react-oidc";
@@ -115,10 +116,42 @@ export function LoginForm({
115116
};
116117

117118
if (isLoading) {
118-
return <div>Loading...</div>;
119+
return (
120+
<Box
121+
sx={{
122+
ml: { xs: "5%", md: "30%" },
123+
mr: { xs: "5%", md: "30%" },
124+
pt: "10%",
125+
}}
126+
>
127+
<Box sx={{ display: "flex", justifyContent: "center", pb: "10%" }}>
128+
<Skeleton
129+
variant="circular"
130+
animation="pulse"
131+
width={150}
132+
height={150}
133+
/>
134+
</Box>
135+
<Skeleton
136+
variant="rectangular"
137+
animation="pulse"
138+
height={56}
139+
sx={{ mb: 4 }}
140+
/>
141+
<Skeleton
142+
variant="rectangular"
143+
animation="pulse"
144+
height={56}
145+
sx={{ mb: 4 }}
146+
/>
147+
<Skeleton variant="rectangular" animation="pulse" height={42} />
148+
</Box>
149+
);
119150
}
120151
if (error) {
121-
return <div>An error occurred while fetching metadata.</div>;
152+
return (
153+
<Alert severity="error">An error occurred while fetching metadata.</Alert>
154+
);
122155
}
123156
if (!metadata) {
124157
return <div>No metadata found.</div>;
@@ -207,25 +240,14 @@ export function LoginForm({
207240
))}
208241
</Select>
209242
</FormControl>
210-
<Stack
211-
direction={{ xs: "column", sm: "row" }}
212-
spacing={2}
243+
<Button
244+
variant="contained"
213245
sx={{ mt: 5, width: "100%" }}
246+
onClick={handleConfigurationChanges}
247+
data-testid="login-form-button"
214248
>
215-
<Button
216-
variant="contained"
217-
sx={{
218-
flexGrow: 1,
219-
}}
220-
onClick={handleConfigurationChanges}
221-
data-testid="login-form-button"
222-
>
223-
Login via your Identity Provider
224-
</Button>
225-
<Button variant="outlined" onClick={() => {}}>
226-
Advanced Options
227-
</Button>
228-
</Stack>
249+
Login via your Identity Provider
250+
</Button>
229251
<Typography
230252
sx={{ paddingTop: "5%", color: "gray", textAlign: "center" }}
231253
>

packages/diracx-web-components/src/contexts/ThemeProvider.tsx

Lines changed: 61 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,10 @@ import {
88
lighten,
99
darken,
1010
alpha,
11-
PaletteMode,
1211
useMediaQuery,
1312
} from "@mui/material";
1413
import { cyan, grey, lightGreen } from "@mui/material/colors";
15-
import { createContext, useMemo, useState } from "react";
14+
import { createContext, useCallback, useMemo, useState } from "react";
1615

1716
declare module "@mui/material/styles" {
1817
interface Palette {
@@ -29,13 +28,21 @@ declare module "@mui/material/styles" {
2928
}
3029
}
3130

31+
/** The two supported theme modes */
32+
export type ThemeMode = "light" | "dark";
33+
34+
/** Type guard narrowing an arbitrary stored string to a valid theme mode */
35+
function isThemeMode(value: string | null): value is ThemeMode {
36+
return value === "light" || value === "dark";
37+
}
38+
3239
/**
3340
* Theme context type
3441
* @property theme - the current theme mode
3542
* @property toggleTheme - function to toggle the theme mode
3643
*/
3744
type ThemeContextType = {
38-
theme: string;
45+
theme: ThemeMode;
3946
toggleTheme: () => void;
4047
};
4148

@@ -58,49 +65,45 @@ export const ThemeContext = createContext<ThemeContextType | undefined>(
5865
*/
5966
export const ThemeProvider = ({ children }: ThemeProviderProps) => {
6067
const prefersDarkMode = useMediaQuery("(prefers-color-scheme: dark)");
61-
const [theme, setTheme] = useState<string | null>(null);
68+
const [theme, setTheme] = useState<ThemeMode | null>(null);
6269
const [prevPrefersDark, setPrevPrefersDark] = useState(prefersDarkMode);
6370

64-
// Initialize theme from sessionStorage or system preference
71+
// Initialize theme from sessionStorage or system preference.
72+
// Render is kept pure: storage is only written from the toggle handler,
73+
// where it records an explicit user choice.
6574
if (theme === null) {
6675
const storedTheme =
6776
typeof sessionStorage !== "undefined"
6877
? sessionStorage.getItem("theme")
6978
: null;
70-
if (storedTheme) {
71-
setTheme(storedTheme);
72-
} else {
73-
const defaultTheme = prefersDarkMode ? "dark" : "light";
74-
setTheme(defaultTheme);
75-
if (typeof sessionStorage !== "undefined") {
76-
sessionStorage.setItem("theme", defaultTheme);
77-
}
78-
}
79+
setTheme(
80+
isThemeMode(storedTheme)
81+
? storedTheme
82+
: prefersDarkMode
83+
? "dark"
84+
: "light",
85+
);
7986
}
8087

81-
// Update theme when system preference changes
88+
// Follow system preference changes unless the user made an explicit choice
8289
if (prevPrefersDark !== prefersDarkMode) {
8390
setPrevPrefersDark(prefersDarkMode);
8491
const storedTheme =
8592
typeof sessionStorage !== "undefined"
8693
? sessionStorage.getItem("theme")
8794
: null;
88-
if (!storedTheme) {
89-
const defaultTheme = prefersDarkMode ? "dark" : "light";
90-
setTheme(defaultTheme);
91-
if (typeof sessionStorage !== "undefined") {
92-
sessionStorage.setItem("theme", defaultTheme);
93-
}
95+
if (!isThemeMode(storedTheme)) {
96+
setTheme(prefersDarkMode ? "dark" : "light");
9497
}
9598
}
9699

97-
const toggleTheme = () => {
100+
const toggleTheme = useCallback(() => {
98101
setTheme((prevTheme) => {
99-
const newTheme = prevTheme === "light" ? "dark" : "light";
102+
const newTheme: ThemeMode = prevTheme === "light" ? "dark" : "light";
100103
sessionStorage.setItem("theme", newTheme);
101104
return newTheme;
102105
});
103-
};
106+
}, []);
104107

105108
const muiTheme = useMemo(() => {
106109
if (theme === null) return createTheme();
@@ -130,10 +133,20 @@ export const ThemeProvider = ({ children }: ThemeProviderProps) => {
130133
? lighten(tableRowFirstColor, 0.2)
131134
: darken(tableRowSecondColor, 0.8);
132135

136+
// Outlined-input border/label colors for the current palette mode.
137+
// Light mode keeps the historical grey[200] derivatives; dark mode
138+
// mirrors them from grey[800] so borders stay subtle on dark surfaces.
139+
const inputBorder = theme === "light" ? grey[200] : grey[800];
140+
const inputBorderHover =
141+
theme === "light" ? darken(grey[200], 0.2) : lighten(grey[800], 0.2);
142+
const inputBorderFocused =
143+
theme === "light" ? darken(grey[200], 0.4) : lighten(grey[800], 0.4);
144+
const inputTextFocused = theme === "light" ? "black" : "white";
145+
133146
// Create a Material-UI theme based on the current mode
134147
const muiTheme = createTheme({
135148
palette: {
136-
mode: theme as PaletteMode,
149+
mode: theme,
137150
primary: {
138151
main: primary,
139152
},
@@ -243,7 +256,7 @@ export const ThemeProvider = ({ children }: ThemeProviderProps) => {
243256
styleOverrides: {
244257
root: {
245258
"&.Mui-focused": {
246-
color: darken(grey[200], 0.4),
259+
color: inputBorderFocused,
247260
},
248261
},
249262
},
@@ -252,14 +265,14 @@ export const ThemeProvider = ({ children }: ThemeProviderProps) => {
252265
styleOverrides: {
253266
root: {
254267
"& .MuiOutlinedInput-notchedOutline": {
255-
borderColor: grey[200],
268+
borderColor: inputBorder,
256269
},
257270
"&:hover .MuiOutlinedInput-notchedOutline": {
258-
borderColor: darken(grey[200], 0.2),
271+
borderColor: inputBorderHover,
259272
},
260273
"&.Mui-focused .MuiOutlinedInput-notchedOutline": {
261-
borderColor: darken(grey[200], 0.4),
262-
color: "black",
274+
borderColor: inputBorderFocused,
275+
color: inputTextFocused,
263276
},
264277
},
265278
},
@@ -363,17 +376,20 @@ export const ThemeProvider = ({ children }: ThemeProviderProps) => {
363376
defaultProps: { size: "small" },
364377
styleOverrides: {
365378
root: {
366-
"& .MuiTableRow-root:nth-of-type(odd)": {
379+
// Use data-attribute selectors instead of nth-of-type so that
380+
// virtualized tables (where DOM order ≠ data order) get stable
381+
// alternating row colors.
382+
"& .MuiTableRow-root[data-row-parity='odd'] td": {
367383
backgroundColor: muiTheme.palette.tableRow.odd,
368-
"&:hover": {
369-
backgroundColor: darken(muiTheme.palette.tableRow.odd, 0.1),
370-
},
371384
},
372-
"& .MuiTableRow-root:nth-of-type(even)": {
385+
"& .MuiTableRow-root[data-row-parity='odd']:hover td": {
386+
backgroundColor: darken(muiTheme.palette.tableRow.odd, 0.1),
387+
},
388+
"& .MuiTableRow-root[data-row-parity='even'] td": {
373389
backgroundColor: muiTheme.palette.tableRow.even,
374-
"&:hover": {
375-
backgroundColor: darken(muiTheme.palette.tableRow.even, 0.1),
376-
},
390+
},
391+
"& .MuiTableRow-root[data-row-parity='even']:hover td": {
392+
backgroundColor: darken(muiTheme.palette.tableRow.even, 0.1),
377393
},
378394
},
379395
},
@@ -383,12 +399,17 @@ export const ThemeProvider = ({ children }: ThemeProviderProps) => {
383399
return muiTheme;
384400
}, [theme]);
385401

386-
if (theme === null) {
402+
const themeContextValue = useMemo<ThemeContextType | null>(
403+
() => (theme === null ? null : { theme, toggleTheme }),
404+
[theme, toggleTheme],
405+
);
406+
407+
if (themeContextValue === null) {
387408
return <div>Loading Theme...</div>;
388409
}
389410

390411
return (
391-
<ThemeContext value={{ theme: theme, toggleTheme }}>
412+
<ThemeContext value={themeContextValue}>
392413
<MUIThemeProvider theme={muiTheme}>
393414
<CssBaseline />
394415
{children}

0 commit comments

Comments
 (0)