Skip to content

Conversation

@tudddorrr
Copy link
Collaborator

No description provided.

tudddorrr and others added 14 commits January 6, 2026 22:40
Bumps [react-router](https://github.com/remix-run/react-router/tree/HEAD/packages/react-router) to 6.30.3 and updates ancestor dependency [react-router-dom](https://github.com/remix-run/react-router/tree/HEAD/packages/react-router-dom). These dependencies need to be updated together.


Updates `react-router` from 6.30.1 to 6.30.3
- [Release notes](https://github.com/remix-run/react-router/releases)
- [Changelog](https://github.com/remix-run/react-router/blob/main/CHANGELOG.md)
- [Commits](https://github.com/remix-run/react-router/commits/react-router@6.30.3/packages/react-router)

Updates `react-router-dom` from 6.30.1 to 6.30.3
- [Release notes](https://github.com/remix-run/react-router/releases)
- [Changelog](https://github.com/remix-run/react-router/blob/main/CHANGELOG.md)
- [Commits](https://github.com/remix-run/react-router/commits/react-router-dom@6.30.3/packages/react-router-dom)

---
updated-dependencies:
- dependency-name: react-router
  dependency-version: 6.30.3
  dependency-type: indirect
- dependency-name: react-router-dom
  dependency-version: 6.30.3
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
…b5c27ca1

Bump react-router and react-router-dom
Bumps [qs](https://github.com/ljharb/qs) and [@cypress/request](https://github.com/cypress-io/request). These dependencies needed to be updated together.

Updates `qs` from 6.14.0 to 6.14.1
- [Changelog](https://github.com/ljharb/qs/blob/main/CHANGELOG.md)
- [Commits](ljharb/qs@v6.14.0...v6.14.1)

Updates `@cypress/request` from 3.0.9 to 3.0.10
- [Release notes](https://github.com/cypress-io/request/releases)
- [Changelog](https://github.com/cypress-io/request/blob/master/CHANGELOG.md)
- [Commits](cypress-io/request@v3.0.9...v3.0.10)

---
updated-dependencies:
- dependency-name: qs
  dependency-version: 6.14.1
  dependency-type: indirect
- dependency-name: "@cypress/request"
  dependency-version: 3.0.10
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Add save content linear mode, limit node width, add copy button
@tudddorrr tudddorrr added the release This pull request contains a release label Jan 12, 2026
@claude
Copy link

claude bot commented Jan 12, 2026

Code Review - PR #443

🔴 Code Quality and Best Practices

Invalid Tailwind CSS class name

In src/components/saves/SaveDataNode.tsx:115, the class wrap-break-word is not a valid Tailwind CSS class. This will not apply any styling.

// Current (incorrect):
className={clsx('text-sm text-white font-mono wrap-break-word', {

// Should be one of:
className={clsx('text-sm text-white font-mono break-words', {  // breaks on word boundaries
// or
className={clsx('text-sm text-white font-mono break-all', {   // breaks anywhere

🟡 Potential Bugs or Issues

Missing null check in clipboard operation

In src/components/saves/SaveDataNode.tsx:91-97, the copyValue function attempts to use navigator.clipboard.writeText() without checking if the Clipboard API is available. This could fail in non-HTTPS contexts or older browsers.

const copyValue = useCallback(() => {
  if (valueRow) {
    const value = valueRow.item.split(': ')[1] || ''
    navigator.clipboard.writeText(value)  // Could throw in non-HTTPS
    toast.trigger('Value copied to clipboard')
  }
}, [valueRow, toast])

Consider adding error handling:

const copyValue = useCallback(() => {
  if (valueRow) {
    const value = valueRow.item.split(': ')[1] || ''
    navigator.clipboard.writeText(value)
      .catch(() => toast.trigger('Failed to copy to clipboard', 'error'))
      .then(() => toast.trigger('Value copied to clipboard'))
  }
}, [valueRow, toast])

Inconsistent Tailwind class syntax

In src/components/RecoveryCodes.tsx:57, the class uses the new Tailwind v4 arbitrary variant syntax items-start\!, but this is inconsistent with the rest of the codebase which uses the older \!items-start syntax. While both may work, consistency is important.

// Line 57:
className='items-start\!'

// Should be (to match existing patterns):
className='\!items-start'

🔵 Performance Considerations

Unnecessary re-computation on every render

In src/pages/PlayerSaveContent.tsx:51-53, both graph hooks are called unconditionally even though only one is used based on the mode. While the enabled flag prevents computation, the hooks still execute and maintain state.

const treeGraph = useNodeGraph(save, search, mode === 'tree')
const linearGraph = useLinearNodeGraph(save, search, mode === 'linear')
const { nodes, edges } = mode === 'tree' ? treeGraph : linearGraph

This is a minor performance consideration but could be optimized by conditionally calling only the needed hook.


Security Concerns

No issues found.

@tudddorrr tudddorrr enabled auto-merge January 12, 2026 23:30
@tudddorrr tudddorrr merged commit 8ea5c0c into main Jan 12, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release This pull request contains a release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants