Skip to content

Commit

Permalink
refactor: Move sizing constants into theme and remove 'any' types
Browse files Browse the repository at this point in the history
- **Removed `constants.ts`**:
  - Deleted `constants.ts` which contained `TITLE_SIZE`, `SUBTITLE_SIZE`, and `TEXT_SIZE`.
- **Centralized Sizing in Theme**:
  - Extended the Material-UI theme by adding a `sizes` property in `theme.ts`.
    - Declared custom theme interfaces to include `sizes` with `title`, `subtitle`, and `text`.
  - Updated components to use `theme.sizes` instead of importing constants.
  - Adjusted imports and styled components accordingly.
- **Improved Type Safety**:
  - Replaced `any` type assertions with more specific types or `unknown` where appropriate.
    - Changed image module declarations in `images.d.ts` from `any` to `string`.
    - Updated test files to use correct type assertions instead of `any`.
    - Added `NumericKeys` utility type in `types.ts` for better type inference.
  - Added ESLint disable comment in `jest.preset.js` to suppress `@typescript-eslint/no-require-imports` warning.
- **Updated Scripts and Documentation**:
  - Renamed `format` script to `lint:prettier:fix` in `package.json` and `README.md` for clarity.
  - Added `lint:eslint` script to run ESLint.
- **Code Style Adjustments**:
  - Modified components to align with updated linting and formatting rules.
  - Ensured consistent usage of `theme.sizes` across all components.
  - Removed unused imports and variables in various files.

These changes improve type safety by eliminating unnecessary `any` types and centralize sizing constants within the theme for consistent styling across the application.
  • Loading branch information
JessicaMulein committed Oct 19, 2024
1 parent eaee25a commit 9be7b51
Show file tree
Hide file tree
Showing 14 changed files with 84 additions and 51 deletions.
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,9 @@ Other commands available:
- yarn test:jest:single: Runs a single specified test (eg yarn test:jest:single src/game/**tests**/dominion-lib-load-save-saveGame.spec.ts)
- yarn test:playwright: Runs playwright e2e tests
- yarn test:playwright:report: Shows the playwright report
- yarn lint:eslint: Runs ESLint
- yarn lint:eslint:fix: Runs ESLint with auto-fix option
- yarn format: Runs Prettier to format various file types
- yarn lint:prettier:fix: Runs Prettier to format various file types
- yarn clean: Removes dist/coverage directories
- yarn reset: Removes node_modules, dist, coverage, and runs yarn install again

Expand Down
1 change: 1 addition & 0 deletions jest.preset.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable @typescript-eslint/no-require-imports */
const nxPreset = require('@nx/jest/preset').default;

module.exports = { ...nxPreset };
6 changes: 3 additions & 3 deletions src/components/IncrementDecrementControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { SxProps } from '@mui/system';
import { Theme } from '@mui/material/styles';
import AddIcon from '@mui/icons-material/Add';
import RemoveIcon from '@mui/icons-material/Remove';
import { SUBTITLE_SIZE } from '@/components/constants';
import theme from '@/components/theme';

// Modify StyledTypography to use forwardRef
const StyledTypography = forwardRef<HTMLSpanElement, TypographyProps>((props, ref) => (
Expand All @@ -13,7 +13,7 @@ const StyledTypography = forwardRef<HTMLSpanElement, TypographyProps>((props, re
{...props}
sx={{
fontFamily: 'TrajanProBold',
fontSize: SUBTITLE_SIZE,
fontSize: theme.sizes.subtitle,
...props.sx,
}}
/>
Expand All @@ -28,7 +28,7 @@ const StyledLargeNumber = forwardRef<HTMLSpanElement, TypographyProps>((props, r
{...props}
sx={{
fontFamily: 'TrajanProBold',
fontSize: SUBTITLE_SIZE,
fontSize: theme.sizes.subtitle,
fontWeight: 'bold',
...props.sx,
}}
Expand Down
19 changes: 10 additions & 9 deletions src/components/Player.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import React, { useState } from 'react';
import { Checkbox, Paper, Box, IconButton, Popover, Tooltip, Typography } from '@mui/material';
import { styled } from '@mui/system';
import { Theme } from '@mui/material/styles';
import ArrowRightIcon from '@mui/icons-material/ArrowRight';
import SettingsIcon from '@mui/icons-material/Settings';
import { useGameContext } from '@/components/GameContext';
import SuperCapsText from '@/components/SuperCapsText';
import IncrementDecrementControl from '@/components/IncrementDecrementControl';
import { TITLE_SIZE } from '@/components/constants';
import { ILogEntry } from '@/game/interfaces/log-entry';
import { updatePlayerField } from '@/game/dominion-lib';
import { addLogEntry, victoryFieldToGameLogAction } from '@/game/dominion-lib-log';
Expand All @@ -16,7 +16,7 @@ import { useAlert } from '@/components/AlertContext';
import { FailedAddLogEntryError } from '@/game/errors/failed-add-log';
import theme from '@/components/theme';

const StyledPaper = styled(Paper)(({ theme }) => ({
const StyledPaper = styled(Paper)(({ theme }: { theme: Theme }) => ({
padding: theme.spacing(2),
margin: theme.spacing(2),
position: 'relative',
Expand Down Expand Up @@ -53,7 +53,7 @@ const Player: React.FC = () => {

if (gameState.selectedPlayerIndex === -1) {
return (
<StyledPaper elevation={3}>
<StyledPaper elevation={3} theme={theme}>
<Typography variant="h6">No player selected</Typography>
</StyledPaper>
);
Expand Down Expand Up @@ -168,6 +168,7 @@ const Player: React.FC = () => {

return (
<StyledPaper
theme={theme}
elevation={3}
style={{
border: isCorrection
Expand All @@ -180,7 +181,7 @@ const Player: React.FC = () => {
<Box mb={2} display="flex" alignItems="center" justifyContent="space-between">
<Box display="flex" alignItems="center">
{isCurrentPlayer && <ArrowRightIcon sx={{ mr: 1 }} />}
<SuperCapsText fontSize={TITLE_SIZE}>{player.name}</SuperCapsText>
<SuperCapsText fontSize={theme.sizes.title}>{player.name}</SuperCapsText>
</Box>
<IconButton onClick={handleNewTurnClick}>
<SettingsIcon />
Expand All @@ -199,7 +200,7 @@ const Player: React.FC = () => {
<ColumnBox>
<CenteredTitle>
<Tooltip title="These values reset every turn">
<SuperCapsText fontSize={TITLE_SIZE}>Turn</SuperCapsText>
<SuperCapsText fontSize={theme.sizes.title}>Turn</SuperCapsText>
</Tooltip>
</CenteredTitle>
<IncrementDecrementControl
Expand Down Expand Up @@ -233,7 +234,7 @@ const Player: React.FC = () => {
<ColumnBox>
<CenteredTitle>
<Tooltip title="These player mat values persist between turns">
<SuperCapsText fontSize={TITLE_SIZE}>Mats</SuperCapsText>
<SuperCapsText fontSize={theme.sizes.title}>Mats</SuperCapsText>
</Tooltip>
</CenteredTitle>
{gameState.options.mats.coffersVillagers && (
Expand Down Expand Up @@ -283,7 +284,7 @@ const Player: React.FC = () => {
<Box sx={{ paddingTop: 2 }}>
<CenteredTitle>
<Tooltip title="Global Mats affect all players and persist between turns">
<SuperCapsText fontSize={TITLE_SIZE}>Global Mats</SuperCapsText>
<SuperCapsText fontSize={theme.sizes.title}>Global Mats</SuperCapsText>
</Tooltip>
</CenteredTitle>
</Box>
Expand All @@ -303,7 +304,7 @@ const Player: React.FC = () => {
<ColumnBox>
<CenteredTitle>
<Tooltip title="Victory points" arrow>
<SuperCapsText fontSize={TITLE_SIZE}>Victory</SuperCapsText>
<SuperCapsText fontSize={theme.sizes.title}>Victory</SuperCapsText>
</Tooltip>
</CenteredTitle>
<IncrementDecrementControl
Expand Down Expand Up @@ -370,7 +371,7 @@ const Player: React.FC = () => {
>
<Box p={2}>
<CenteredTitle>
<SuperCapsText fontSize={TITLE_SIZE}>Next Turn</SuperCapsText>
<SuperCapsText fontSize={theme.sizes.title}>Next Turn</SuperCapsText>
</CenteredTitle>
<IncrementDecrementControl
label="Actions"
Expand Down
39 changes: 24 additions & 15 deletions src/components/Scoreboard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,28 +9,30 @@ import {
Paper,
Button,
} from '@mui/material';
import { Theme } from '@mui/material/styles';
import { styled } from '@mui/system';
import SuperCapsText from '@/components/SuperCapsText';
import { calculateVictoryPoints } from '@/game/dominion-lib';
import { SUBTITLE_SIZE, TEXT_SIZE } from '@/components/constants';
import ArrowRightIcon from '@mui/icons-material/ArrowRight';
import { useGameContext } from '@/components/GameContext';
import { addLogEntry } from '@/game/dominion-lib-log';
import { GameLogActionWithCount } from '@/game/enumerations/game-log-action-with-count';
import theme from '@/components/theme';

const StyledTableCell = styled(TableCell)(({ theme }) => ({
const StyledTableCell = styled(TableCell)(({ theme }: { theme: Theme }) => ({
fontFamily: 'TrajanProBold',
fontSize: TEXT_SIZE,
fontSize: theme.sizes.text,
}));

const StyledScoreCell = styled(TableCell)(({ theme }) => ({
const StyledScoreCell = styled(TableCell)(({ theme }: { theme: Theme }) => ({
fontFamily: 'TrajanProBold',
fontSize: SUBTITLE_SIZE,
fontSize: theme.sizes.title,
fontWeight: 'bold',
}));

const StyledButton = styled(Button)(({ theme }) => ({
const StyledButton = styled(Button)(({ theme }: { theme: Theme }) => ({
fontFamily: 'TrajanProBold',
fontSize: theme.sizes.text,
}));

const Scoreboard: React.FC = () => {
Expand Down Expand Up @@ -60,10 +62,14 @@ const Scoreboard: React.FC = () => {
<Table>
<TableHead>
<TableRow>
<StyledTableCell>Current</StyledTableCell>
<StyledTableCell>Player</StyledTableCell>
<StyledTableCell align="right">Score</StyledTableCell>
<StyledTableCell align="right">Turn: {gameState.currentTurn}</StyledTableCell>
<StyledTableCell theme={theme}>Current</StyledTableCell>
<StyledTableCell theme={theme}>Player</StyledTableCell>
<StyledTableCell theme={theme} align="right">
Score
</StyledTableCell>
<StyledTableCell theme={theme} align="right">
Turn: {gameState.currentTurn}
</StyledTableCell>
</TableRow>
</TableHead>
<TableBody>
Expand All @@ -76,17 +82,20 @@ const Scoreboard: React.FC = () => {
'&:hover': { backgroundColor: 'rgba(0, 0, 0, 0.04)' },
}}
>
<StyledTableCell>
<StyledTableCell theme={theme}>
{index === getCurrentPlayerIndex() && (
<ArrowRightIcon color="primary" style={{ fontSize: 24 }} />
)}
</StyledTableCell>
<StyledTableCell component="th" scope="row">
<SuperCapsText fontSize={TEXT_SIZE}>{player.name}</SuperCapsText>
<StyledTableCell theme={theme} component="th" scope="row">
<SuperCapsText fontSize={theme.sizes.text}>{player.name}</SuperCapsText>
</StyledTableCell>
<StyledScoreCell align="right">{calculateVictoryPoints(player)}</StyledScoreCell>
<StyledTableCell align="right">
<StyledScoreCell theme={theme} align="right">
{calculateVictoryPoints(player)}
</StyledScoreCell>
<StyledTableCell theme={theme} align="right">
<StyledButton
theme={theme}
variant="contained"
size="small"
onClick={() => handlePlayerSelect(index)}
Expand Down
3 changes: 0 additions & 3 deletions src/components/constants.ts

This file was deleted.

8 changes: 4 additions & 4 deletions src/components/screens/AboutScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import {
ListItemText,
} from '@mui/material';
import DominionTransparentLogo from '@/assets/images/Dominion-tx.png';
import SuperCapsText from '../SuperCapsText';
import { TITLE_SIZE } from '../constants';
import SuperCapsText from '@/components/SuperCapsText';
import theme from '@/components/theme';

export default function AboutScreen() {
return (
Expand Down Expand Up @@ -60,7 +60,7 @@ export default function AboutScreen() {
<Box sx={{ display: 'flex', flexDirection: { xs: 'column', md: 'row' }, gap: 4 }}>
<Box sx={{ flex: 1 }}>
<Paper elevation={3} sx={{ p: 2, height: '100%' }}>
<SuperCapsText fontSize={TITLE_SIZE}>Features</SuperCapsText>
<SuperCapsText fontSize={theme.sizes.title}>Features</SuperCapsText>
<List dense>
{[
'Player Management: Add, remove, and track multiple players',
Expand All @@ -82,7 +82,7 @@ export default function AboutScreen() {

<Box sx={{ flex: 1 }}>
<Paper elevation={3} sx={{ p: 2, height: '100%' }}>
<SuperCapsText fontSize={TITLE_SIZE} sx={{ paddingBottom: '10px' }}>
<SuperCapsText fontSize={theme.sizes.title} sx={{ paddingBottom: '10px' }}>
About
</SuperCapsText>
<Typography variant="body1" paragraph>
Expand Down
7 changes: 1 addition & 6 deletions src/components/screens/DominionAssistantScreen.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import React from 'react';
import { Box, Typography } from '@mui/material';
import { Box } from '@mui/material';
import { styled } from '@mui/system';
import { useNavigate, useLocation } from 'react-router-dom';
import DominionAssistant from '@/components/DominionAssistant';
Expand All @@ -13,11 +13,6 @@ const StyledContainer = styled(Box)(({ theme }) => ({
height: '100vh',
}));

const StyledTitle = styled(Typography)(({ theme }) => ({
fontFamily: 'CharlemagneStdBold',
marginBottom: theme.spacing(2),
}));

export default function DominionAssistantScreen() {
const navigate = useNavigate();
const location = useLocation();
Expand Down
4 changes: 0 additions & 4 deletions src/components/screens/LoadSaveScreen.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
import React from 'react';
import { Box } from '@mui/material';
import { useLocation, useNavigate } from 'react-router-dom';
import LoadSaveGame from '@/components/LoadSaveGame';
import SaveIcon from '@mui/icons-material/Save';

export default function LoadSaveGameScreen() {
const location = useLocation();
const navigate = useNavigate();

return (
<Box
sx={{
Expand Down
22 changes: 22 additions & 0 deletions src/components/theme.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,22 @@
import { createTheme, Theme } from '@mui/material/styles';

declare module '@mui/material/styles' {
interface Theme {
sizes: {
title: number;
subtitle: number;
text: number;
};
}
interface ThemeOptions {
sizes?: {
title?: number;
subtitle?: number;
text?: number;
};
}
}

const theme: Theme = createTheme({
palette: {
primary: {
Expand All @@ -14,6 +31,11 @@ const theme: Theme = createTheme({
fontSize: '1.25rem',
},
},
sizes: {
title: 24,
subtitle: 18,
text: 16,
},
});

export default theme;
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ describe('restoreSavedGame', () => {
const validGameRaw: IGameRaw = createMockGameRaw(2, { log: [validLogEntryRaw] });

it('should throw an error when the game object is invalid', () => {
const invalidGame = { ...validGameRaw, players: null as any }; // Invalid players
const invalidGame = { ...validGameRaw, players: null as unknown as IGameRaw['players'] }; // Invalid players

expect(() => restoreSavedGame(invalidGame)).toThrow('Invalid game object');
});
Expand Down
9 changes: 8 additions & 1 deletion src/game/__tests__/dominion-lib-updatePlayerField.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { EmptyGameState } from '@/game/dominion-lib';
import { InvalidFieldError } from '@/game/errors/invalid-field';
import { NotEnoughSupplyError } from '@/game/errors/not-enough-supply';
import { NotEnoughSubfieldError } from '@/game/errors/not-enough-subfield';
import { PlayerFieldMap } from '@/game/types';

describe('updatePlayerField', () => {
let mockGame: IGame;
Expand Down Expand Up @@ -69,7 +70,13 @@ describe('updatePlayerField', () => {

it('should throw InvalidFieldError for non-existent fields', () => {
expect(() => {
updatePlayerField(mockGame, 0, 'nonexistent' as any, 'subfield' as any, 1);
updatePlayerField(
mockGame,
0,
'nonexistentField' as keyof PlayerFieldMap,
'subfield' as unknown as keyof IPlayer['turn'],
1
);
}).toThrow(InvalidFieldError);
});

Expand Down
4 changes: 4 additions & 0 deletions src/game/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,7 @@ export type OptionField = keyof IGameOptions;
export type OptionSubField<T extends OptionField> = T extends 'curses'
? boolean
: keyof IGameOptions[T];

export type NumericKeys<T> = {
[K in keyof T]: T[K] extends number ? K : never;
}[keyof T];
8 changes: 4 additions & 4 deletions src/types/images.d.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
declare module '*.png' {
const content: any;
const content: string;
export default content;
}

declare module '*.jpg' {
const content: any;
const content: string;
export default content;
}

declare module '*.jpeg' {
const content: any;
const content: string;
export default content;
}

declare module '*.svg' {
const content: any;
const content: string;
export default content;
}

0 comments on commit 9be7b51

Please sign in to comment.