Repository navigation
Add reusable component comparison mode - #17
Conversation
|
@rheos is attempting to deploy a commit to the Yoseph's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Ready for maintainer review. Implementation and final verification are complete: type checking, linting, all 57 tests, production build, cross-browser interaction QA, accessibility/reduced-motion checks, and repeated WebGL lifecycle testing all passed. The only remaining failed status is Vercel deployment authorization. CI/CD is maintainer-owned, so no deployment configuration was changed in this PR. |
|
This looks really solid and the GPU comparison itself is working well. One thing before I merge it: the original scope of #8 was comparison between components within the same category in general, not only GPUs. Would you be willing to generalize the comparison system so it can support the other categories as well? If that’s significantly more work, I’m also fine treating this as Phase 1, but in that case I’d like to keep #8 open and track the remaining generic comparison work separately rather than closing it with this PR. Also, Is the comparison infrastructure already generic and the UI currently just exposes GPUs, or is the underlying implementation itself GPU-specific? Because I see there has been 2554 additions across 15 files. |
|
Follow-up on support for future models and categories: I changed this in commit That means a new GPU, power supply, or processor only needs to be added to the normal level/model/catalogue setup and assigned to its comparison group. The comparison view and renderer do not need to change. A completely new category still needs a small schema describing which specifications should appear in its table, since those fields differ by category. It then uses the same comparison UI, state handling, and renderer. The discovery behavior is covered by the comparison tests, with all 58 tests passing locally. |
|
Thanks, this is solid work, and the checks pass. Before merging, I’d like the comparison mode to fully satisfy #8 by supporting every category with at least two comparable models, rather than only GPU, PSU, and CPU. In particular, please add Storage comparison for the SATA SSD and NVMe models. Their current model scales differ, so these must first be normalized to ensure the physical-size comparison remains accurate. Please also include relevant dimensions, interfaces, architecture, and category-specific specifications wherever applicable. Category-specific specification definitions may still require configuration, since the meaningful fields differ between component types. Finally, could you reduce the implementation by roughly 400–600 lines without removing functionality or changing the visible UI?
Also to be specific i’m not asking you to replace the shared renderer architecture,it is well designed, perhaps the button placement where swapping between different categories could be improved and/or made larger and more obvious. The goal is to retain the same behavior and appearance while making the PR easier to maintain. Please also update the inconsistent test counts in with the new techincalities added ARCHITECTURE.md. Please do leave a comment with what has been updated for the next push here in the thread. |
|
Changes completed before merging:
|
Closes #8
Summary
Adding models and categories
The comparison view no longer keeps its own hardcoded list of models. Each comparable model declares a
comparisonGroupin the main level catalogue, and the shared workbench discovers the eligible models and their catalogue data from there.Adding another GPU, power supply, or processor therefore requires no change to the comparison view. A completely new category only needs a small schema describing the specifications that should appear in its table, since those fields differ by category. It still uses the same comparison UI and renderer.
Automated verification
npm run checknpm run lintnpm test— 58/58 passing, including metadata-driven model discovery, every configured group, and common bounds at assembled, half-disassembled, and inventory positionsnpm run build— passing (existing chunk-size advisory only)git diff --checkBrowser QA
Ready for maintainer review. The latest GitHub Actions run is waiting for maintainer approval because it comes from a fork. Vercel also reports an external deployment-authorization failure. CI/CD is maintainer-owned and was intentionally left unchanged.