feat(rules page): rebuild directory rules page with vendored lists#1147
Merged
Conversation
Mirror per-directory JSON from lists into src/data/5chan-directories, rework /rules layout to match 4chan (sidebar nav, category boxes, P2P load), and keep spoiler markup visible in rule text via parseSpoilers.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR mirrors per-directory JSON from GitHub (with validation and pruning), adds a vendored assembly module, persists and exposes directory defaults via hooks, refactors the Rules page to use directory defaults and a P2P loader with deep-link scrolling, updates styles and tests, and adds optional spoiler parsing to Markdown. ChangesDirectory Data Infrastructure Refactor and Rules UI Overhaul
Markdown Spoiler Parsing Control
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/views/rules/__tests__/rules.test.tsx (1)
34-42: ⚡ Quick winRoute tests still bypass React Router's param parsing.
Because
useParams()is mocked here, the deep-link assertions don't prove that/rules/:boardIdentifieris actually wired correctly. For this feature, it's worth rendering through a real<Route>and driving the param viaMemoryRouter.initialEntries.Suggested test shape
-import { MemoryRouter } from 'react-router-dom'; +import { MemoryRouter, Route, Routes } from 'react-router-dom';vi.mock('react-router-dom', async () => { - const actual = await vi.importActual<typeof import('react-router-dom')>('react-router-dom'); - return { - ...actual, - useParams: () => ({ - boardIdentifier: testState.boardIdentifier, - }), - }; + return vi.importActual<typeof import('react-router-dom')>('react-router-dom'); });-const renderRules = async () => { +const renderRules = async (initialEntry = '/rules') => { await act(async () => { - root.render(createElement(MemoryRouter, null, createElement(Rules))); + root.render( + createElement( + MemoryRouter, + { initialEntries: [initialEntry] }, + createElement( + Routes, + null, + createElement(Route, { path: '/rules/:boardIdentifier?', element: createElement(Rules) }), + ), + ), + ); }); };- testState.boardIdentifier = 'a'; - await renderRules(); + await renderRules('/rules/a');As per coding guidelines, "Add or update tests for bug fixes and non-trivial logic changes when the code is reasonably testable."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/rules/__tests__/rules.test.tsx` around lines 34 - 42, The test currently mocks useParams which bypasses React Router; remove or stop mocking useParams in this test and instead render the component through a real router by wrapping the tested component in a MemoryRouter with initialEntries set to [`/rules/${testState.boardIdentifier}`] and a Route path="/rules/:boardIdentifier" so params are parsed naturally; update the test render call (the render import from `@testing-library/react`) to use <MemoryRouter initialEntries={...}><Routes><Route path="/rules/:boardIdentifier" element={<YourComponent />} /></Routes></MemoryRouter>, drive assertions against the rendered output and keep references to testState.boardIdentifier and any existing render/test utilities (e.g., render, Routes, Route, MemoryRouter) so the deep-link behavior is validated without mocking useParams.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/sync-directories.js`:
- Around line 127-128: The warning hardcodes "from GitHub" which hides the real
source; change the catch block message to include the actual source by building
a source description (e.g., use DIRECTORIES_SOURCE_PATH when present, otherwise
compose the GitHub identifier like `${GITHUB_REPO}@${GITHUB_BRANCH}`) and
interpolate that into the process message instead of the fixed "from GitHub"
text so failures show the real command/URL/path being acted on.
In `@src/hooks/use-directories.ts`:
- Around line 477-497: Remove the local useState/useEffect and GitHub refresh
from useDirectoryDefaults; instead call the shared directories hook (e.g.,
invoke useDirectories() or useDirectoriesState() inside useDirectoryDefaults)
and simply return cacheDefaults ?? getFallbackDirectoryDefaults(); delete the
isMounted wrapper and the fetchDirectoriesFromGitHubDeduped() call so defaults
are derived from the shared cache (cacheDefaults) and no duplicate fetch path is
created.
In `@src/views/rules/rules.tsx`:
- Around line 255-270: The effect currently always calls window.scrollTo(0, 0)
whenever boardIdentifier is falsy (e.g., on /rules) even if directories change;
change the no-board branch to only clear and scroll when the previous state
indicated we had scrolled to a specific board. Concretely, inside the useEffect
that depends on [boardIdentifier, directories], update the if (!boardIdentifier)
branch to check scrolledForRef.current and only call window.scrollTo(0, 0) and
set scrolledForRef.current = null when scrolledForRef.current !== null; keep the
early return otherwise so directory refreshes no longer yank readers to the top.
---
Nitpick comments:
In `@src/views/rules/__tests__/rules.test.tsx`:
- Around line 34-42: The test currently mocks useParams which bypasses React
Router; remove or stop mocking useParams in this test and instead render the
component through a real router by wrapping the tested component in a
MemoryRouter with initialEntries set to [`/rules/${testState.boardIdentifier}`]
and a Route path="/rules/:boardIdentifier" so params are parsed naturally;
update the test render call (the render import from `@testing-library/react`) to
use <MemoryRouter initialEntries={...}><Routes><Route
path="/rules/:boardIdentifier" element={<YourComponent />}
/></Routes></MemoryRouter>, drive assertions against the rendered output and
keep references to testState.boardIdentifier and any existing render/test
utilities (e.g., render, Routes, Route, MemoryRouter) so the deep-link behavior
is validated without mocking useParams.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 50196014-b6b1-4873-933d-15bbc218d767
📒 Files selected for processing (73)
scripts/sync-directories.jssrc/components/markdown/__tests__/markdown.test.tsxsrc/components/markdown/markdown.tsxsrc/data/5chan-directories/5chan-3-directory.jsonsrc/data/5chan-directories/5chan-a-directory.jsonsrc/data/5chan-directories/5chan-adv-directory.jsonsrc/data/5chan-directories/5chan-an-directory.jsonsrc/data/5chan-directories/5chan-b-directory.jsonsrc/data/5chan-directories/5chan-bant-directory.jsonsrc/data/5chan-directories/5chan-biz-directory.jsonsrc/data/5chan-directories/5chan-c-directory.jsonsrc/data/5chan-directories/5chan-ck-directory.jsonsrc/data/5chan-directories/5chan-co-directory.jsonsrc/data/5chan-directories/5chan-directories-defaults.jsonsrc/data/5chan-directories/5chan-diy-directory.jsonsrc/data/5chan-directories/5chan-f-directory.jsonsrc/data/5chan-directories/5chan-fa-directory.jsonsrc/data/5chan-directories/5chan-fit-directory.jsonsrc/data/5chan-directories/5chan-g-directory.jsonsrc/data/5chan-directories/5chan-gd-directory.jsonsrc/data/5chan-directories/5chan-gif-directory.jsonsrc/data/5chan-directories/5chan-his-directory.jsonsrc/data/5chan-directories/5chan-i-directory.jsonsrc/data/5chan-directories/5chan-ic-directory.jsonsrc/data/5chan-directories/5chan-int-directory.jsonsrc/data/5chan-directories/5chan-jp-directory.jsonsrc/data/5chan-directories/5chan-k-directory.jsonsrc/data/5chan-directories/5chan-lit-directory.jsonsrc/data/5chan-directories/5chan-m-directory.jsonsrc/data/5chan-directories/5chan-mlp-directory.jsonsrc/data/5chan-directories/5chan-mu-directory.jsonsrc/data/5chan-directories/5chan-n-directory.jsonsrc/data/5chan-directories/5chan-news-directory.jsonsrc/data/5chan-directories/5chan-o-directory.jsonsrc/data/5chan-directories/5chan-out-directory.jsonsrc/data/5chan-directories/5chan-p-directory.jsonsrc/data/5chan-directories/5chan-po-directory.jsonsrc/data/5chan-directories/5chan-pol-directory.jsonsrc/data/5chan-directories/5chan-pw-directory.jsonsrc/data/5chan-directories/5chan-qst-directory.jsonsrc/data/5chan-directories/5chan-r9k-directory.jsonsrc/data/5chan-directories/5chan-s5s-directory.jsonsrc/data/5chan-directories/5chan-sci-directory.jsonsrc/data/5chan-directories/5chan-soc-directory.jsonsrc/data/5chan-directories/5chan-sp-directory.jsonsrc/data/5chan-directories/5chan-t-directory.jsonsrc/data/5chan-directories/5chan-tg-directory.jsonsrc/data/5chan-directories/5chan-toy-directory.jsonsrc/data/5chan-directories/5chan-trv-directory.jsonsrc/data/5chan-directories/5chan-tv-directory.jsonsrc/data/5chan-directories/5chan-v-directory.jsonsrc/data/5chan-directories/5chan-vg-directory.jsonsrc/data/5chan-directories/5chan-vip-directory.jsonsrc/data/5chan-directories/5chan-vm-directory.jsonsrc/data/5chan-directories/5chan-vmg-directory.jsonsrc/data/5chan-directories/5chan-vp-directory.jsonsrc/data/5chan-directories/5chan-vr-directory.jsonsrc/data/5chan-directories/5chan-vrpg-directory.jsonsrc/data/5chan-directories/5chan-vst-directory.jsonsrc/data/5chan-directories/5chan-vt-directory.jsonsrc/data/5chan-directories/5chan-w-directory.jsonsrc/data/5chan-directories/5chan-wg-directory.jsonsrc/data/5chan-directories/5chan-wsg-directory.jsonsrc/data/5chan-directories/5chan-wsr-directory.jsonsrc/data/5chan-directories/5chan-x-directory.jsonsrc/data/5chan-directories/5chan-xs-directory.jsonsrc/data/5chan-directory-lists.jsonsrc/data/vendored-directory-lists.tssrc/hooks/use-directories.tssrc/lib/utils/directory-list-lookup-utils.tssrc/views/rules/__tests__/rules.test.tsxsrc/views/rules/rules.module.csssrc/views/rules/rules.tsx
💤 Files with no reviewable changes (1)
- src/data/5chan-directory-lists.json
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/hooks/use-directories.ts (1)
477-500: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
useDirectoryDefaults()still fetches inside auseEffect.This hook adds a data-fetching
useEffect(thefetchDirectoriesFromGitHubDeduped()call plus thedefaults/setDefaultsstate that only mirrorscacheDefaults). Defaults can be derived from the shared directories hook instead, e.g. depend onuseDirectories()/useDirectoriesState()and returncacheDefaults ?? getFallbackDirectoryDefaults(), dropping the effect and local state.As per coding guidelines: "Do not use
useEffectfor data fetching; usebitsocial-react-hooks" and "Do not sync derived state with effects; compute during render".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/use-directories.ts` around lines 477 - 500, The hook useDirectoryDefaults currently creates local state and a useEffect that calls fetchDirectoriesFromGitHubDeduped and mirrors cacheDefaults into defaults; remove the local useState, the useEffect, and the fetchDirectoriesFromGitHubDeduped call, and instead derive the value directly from the shared directories state by importing and using useDirectories() or useDirectoriesState() (so fetching is handled by the shared hook); simplify the function to return cacheDefaults ?? getFallbackDirectoryDefaults() (or cacheDefaults ?? getFallbackDirectoryDefaults() combined with any value read from useDirectories/useDirectoriesState() as needed) and drop isMounted/setDefaults logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/hooks/use-directories.ts`:
- Around line 477-500: The hook useDirectoryDefaults currently creates local
state and a useEffect that calls fetchDirectoriesFromGitHubDeduped and mirrors
cacheDefaults into defaults; remove the local useState, the useEffect, and the
fetchDirectoriesFromGitHubDeduped call, and instead derive the value directly
from the shared directories state by importing and using useDirectories() or
useDirectoriesState() (so fetching is handled by the shared hook); simplify the
function to return cacheDefaults ?? getFallbackDirectoryDefaults() (or
cacheDefaults ?? getFallbackDirectoryDefaults() combined with any value read
from useDirectories/useDirectoriesState() as needed) and drop
isMounted/setDefaults logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 14aec99e-b5e7-4a25-a849-08dddaac23c3
📒 Files selected for processing (3)
scripts/smoke-web-app.jssrc/hooks/__tests__/use-directories.test.tsxsrc/hooks/use-directories.ts
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 51bd0d2. Configure here.
Member
Author
|
Addressed the valid review findings in follow-up commits:\n\n- kept directory defaults and directory payload refresh/cache updates atomic, including reload fallback from localStorage\n- derived directory defaults through the shared directory refresh path instead of a duplicate fetch effect\n- updated the rules smoke assertion for the new heading\n- cleared loaded P2P rules when navigating between rules routes\n- avoided re-scrolling bare /rules on directory refresh\n- made the directory sync warning name the actual source path or GitHub folder\n\nLocal verification after the last review-driven change: test, lint, type-check, build, doctor, smoke, and cross-engine playwright-cli checks for Chrome/Firefox/WebKit desktop/mobile plus a throttled Chromium pass. Hosted checks are green; review threads are resolved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
5chan-directory-lists.jsonvendored snapshot with a mirroredsrc/data/5chan-directories/folder synced from bitsocialnet/lists/rulesto match 4chan’s layout: green sidebar directory nav, category rule boxes, compact Load/Go controls, and mobile-friendly list spacingparseSpoilers={false}on rules markdown so instructional[spoiler]…[/spoiler]examples render literallyTest plan
/ruleson desktop and confirm sidebar width, typography, and category navigation match 4chan/rules/aand confirm Anime & Manga spoiler syntax shows as plain text, not hidden spoiler UI/ruleson a mobile viewport: bullets visible, adequate left padding, desktop font sizeyarn build,yarn lint,yarn type-check, rules + markdown tests passNote
Medium Risk
Large offline directory data path change affects board resolution when GitHub is down; Rules and defaults caching are user-facing but covered by new tests.
Overview
Replaces the single vendored
5chan-directory-lists.jsonsnapshot with asrc/data/5chan-directories/mirror of upstream lists plusvendored-directory-lists.ts, which merges per-directory files with5chan-directories-defaults.jsonat build time.sync-directories.jsnow copies JSON verbatim (validates before write, prunes removed files) instead of normalizing into one blob.use-directoriesgainsuseDirectoryDefaults, a separate defaults localStorage cache, and keeps defaults in step with GitHub refresh. The Rules page is rebuilt in a 4chan-style layout: sidebar directory nav, Image vs Upload rule sections from defaults, deep links to/rules/:code, and optional P2P rules via address input only (dropdown removed).MarkdownaddsparseSpoilers(off on rules so[spoiler]examples stay visible).Reviewed by Cursor Bugbot for commit 658d928. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
Chores