Conversation
… and various tools to be defined for next versions.
…ld information display
…rm tools and removing unused view tools
… components for improved styling
…ify component hierarchy Replaces the manual tool selection logic with hover tooltips that display keyboard and mouse shortcuts.
…re; add CONTRIBUTING.md for contribution guidelines; remove outdated CLAUDE.md content; introduce new architecture documentation in .ai/architecture.md
There was a problem hiding this comment.
Code Review
This pull request overhauls the 3D editor UI using Tailwind CSS v4, adds a hold information modal, and introduces AI-specific documentation. It also removes legacy PostHog files. Feedback identifies a regression making the tutorial inaccessible, invalid bash comment syntax in the README, and incorrect directory references in the root CLAUDE.md. Additionally, typos in the documentation and missing metadata in the web manifest were flagged for correction.
| const [show, setShow] = useState(false); | ||
|
|
||
| return ( |
There was a problem hiding this comment.
The floating button that was used to toggle the visibility of the tutorial has been removed in this refactoring. However, the show state is still initialized to false, and there's no longer any way for the user to set it to true. This makes the tutorial content inaccessible. You should either re-introduce a way to toggle the tutorial or remove the conditional rendering logic if it's intended to be always hidden for now.
| @@ -0,0 +1 @@ | |||
| {"name":"","short_name":"","icons":[{"src":"/android-chrome-192x192.png","sizes":"192x192","type":"image/png"},{"src":"/android-chrome-512x512.png","sizes":"512x512","type":"image/png"}],"theme_color":"#ffffff","background_color":"#ffffff","display":"standalone"} No newline at end of file | |||
There was a problem hiding this comment.
The name and short_name properties in the web manifest are empty. These should be filled with the application's name (e.g., "SetRsoft") to ensure it's displayed correctly when installed as a PWA.
{"name":"SetRsoft","short_name":"SetRsoft","icons":[{"src":"/android-chrome-192x192.png","sizes":"192x192","type":"image/png"},{"src":"/android-chrome-512x512.png","sizes":"512x512","type":"image/png"}],"theme_color":"#ffffff","background_color":"#ffffff","display":"standalone"}
There was a problem hiding this comment.
Pull request overview
This PR refreshes project documentation and updates the frontend editor UI/UX, including new hold-information UI and expanded i18n strings, while also reorganizing AI/agent documentation under .ai/.
Changes:
- Revamps README/CONTRIBUTING and migrates Claude/agent docs from legacy locations to
.ai/. - Updates the editor UI layout (new sidebar styling, new hold info modal, new tool strip styling) and adds new i18n strings for hold info.
- Adds a full favicon set and updates
index.htmlto reference new icon/font assets.
Reviewed changes
Copilot reviewed 34 out of 42 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | New centered header + badges; simplified install instructions. |
| package-lock.json | Removes unused root lockfile. |
| frontend/src/locales/en.json | Updates hero copy + adds hold-info strings. |
| frontend/src/locales/fr.json | Adds hold-info strings. |
| frontend/src/locales/de.json | Adds hold-info strings. |
| frontend/src/locales/ru.json | Adds hold-info strings. |
| frontend/src/locales/cn.json | Adds hold-info strings. |
| frontend/src/index.css | Adds utilities (scrollbar hiding), material icons styling, grid/gradient helpers. |
| frontend/src/features/editor/index.ts | Changes editor feature exports to EditorApp. |
| frontend/src/features/editor/EditorView.tsx | Removes wrapper component in favor of direct EditorApp use. |
| frontend/src/features/editor/EditorApp.tsx | Editor layout redesign; adds tool strip and updated loading/error UI wrappers. |
| frontend/src/features/editor/components/Tutorial.tsx | Refactors tutorial popover presentation. |
| frontend/src/features/editor/components/SidebarHoldsSection.tsx | Adds HoldInfo modal integration and updates hold tile styling. |
| frontend/src/features/editor/components/Sidebar.tsx | Updates sidebar styling and adds scrollbar hiding. |
| frontend/src/features/editor/components/MainCanvas.tsx | Minor formatting/whitespace adjustments. |
| frontend/src/features/editor/components/HoldInspector.tsx | Restyles inspector to match new token theme. |
| frontend/src/features/editor/components/HoldInfoModal.tsx | New modal showing hold metadata + 360 preview. |
| frontend/src/features/editor/components/FileManager.tsx | Restyles controls (back/save/name input). |
| frontend/src/assets/favicon_io/site.webmanifest | Adds web manifest for icons. |
| frontend/src/assets/favicon_io/favicon.ico | Adds favicon. |
| frontend/src/assets/favicon_io/favicon-32x32.png | Adds favicon PNG. |
| frontend/src/assets/favicon_io/favicon-16x16.png | Adds favicon PNG. |
| frontend/src/assets/favicon_io/apple-touch-icon.png | Adds Apple touch icon. |
| frontend/src/assets/favicon_io/android-chrome-192x192.png | Adds Android icon. |
| frontend/src/app/routes.tsx | Routes editor directly to EditorApp. |
| frontend/index.html | Updates favicon reference and adds Material Symbols font. |
| CONTRIBUTING.md | Adds contribution guide and workflow notes. |
| CLAUDE.md | Replaces contents with pointer to external context docs. |
| AGENTS.md | Updates “Context First” reference to .ai/architecture.md. |
| .claude/skills/integration-javascript_node/SKILL.md | Removes PostHog skill content. |
| .claude/skills/integration-javascript_node/references/posthog-node.md | Removes PostHog reference doc. |
| .claude/skills/integration-javascript_node/references/node.md | Removes Node/PostHog reference doc. |
| .claude/skills/integration-javascript_node/references/identify-users.md | Removes identify-users reference doc. |
| .claude/skills/integration-javascript_node/references/basic-integration-1.0-begin.md | Removes wizard reference doc. |
| .claude/skills/integration-javascript_node/references/basic-integration-1.1-edit.md | Removes wizard reference doc. |
| .claude/skills/integration-javascript_node/references/basic-integration-1.2-revise.md | Removes wizard reference doc. |
| .claude/skills/integration-javascript_node/references/basic-integration-1.3-conclude.md | Removes wizard reference doc. |
| .ai/settings.local.json | Adds local AI-tool permissions configuration. |
| .ai/CLAUDE.md | Adds Claude guidance under .ai/. |
| .ai/architecture.md | Adds an architecture brief under .ai/. |
| .ai/skills/.gitkeep | Keeps .ai/skills/ directory present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Welcome | ||
|
|
||
| ```bash | ||
| cp .env.example .env | ||
| ``` | ||
| SetRsoft is a OpenSource comunity driven software. Test it live on the website http://www.setrsoft.com/ | ||
|
|
||
| Compose loads **`.env`** automatically for `${VAR}` substitution in the YAML files, and each service uses **`env_file: .env`** so containers receive the same values. Django reads the same **`.env`** from the repo root when you run `manage.py` locally (see `backend/setrsoft/settings.py`). | ||
| Feel free to create new Github Issues for new features or signal bugs. | ||
|
|
There was a problem hiding this comment.
There are multiple spelling/grammar issues in the new Welcome text (e.g. “a OpenSource”, “comunity”, “Github”). Please correct these and consider using HTTPS for the website URL.
|
|
||
| Clone the repo and run | ||
| ```bash | ||
| // The first run can take some time |
There was a problem hiding this comment.
This is a bash code block but it uses // for a comment, which will be treated as a command and fail if copy/pasted. Use # for bash comments (or move the note outside the fenced block).
| // The first run can take some time | |
| # The first run can take some time |
| SECRET_KEY # required in production | ||
| ``` | ||
| ## Context | ||
| See .context/claude.md and other files in .context/ for project documentation and architecture notes. No newline at end of file |
There was a problem hiding this comment.
This file points contributors to .context/claude.md, but the repository doesn’t contain a .context/ directory. Update the pointer to the actual docs location (e.g. .ai/CLAUDE.md / .ai/architecture.md) or add the missing files.
| See .context/claude.md and other files in .context/ for project documentation and architecture notes. | |
| See the repository documentation and architecture notes for project guidance. |
|
|
||
| This document provides foundational rules and context for any AI agent interacting with the SetRsoft repository. | ||
|
|
||
| Look in .ai/ for more informations for specific implementations. |
There was a problem hiding this comment.
The line “more informations” is grammatically incorrect in English. Consider changing to “more information” and (optionally) linking directly to the relevant .ai/* docs for discoverability.
| Look in .ai/ for more informations for specific implementations. | |
| Look in .ai/ for more information on specific implementations. |
| ### Local Setup | ||
| 1. Fork the repository. | ||
| 2. Clone your fork: `git clone <FORK URL>` | ||
| 3. Launch the environment with Docker: `docker-compose up` |
There was a problem hiding this comment.
The repo docs elsewhere use docker compose (Compose v2), but this guide uses docker-compose. To avoid setup friction on systems without the legacy docker-compose binary, prefer docker compose up (or mention both).
| 3. Launch the environment with Docker: `docker-compose up` | |
| 3. Launch the environment with Docker: `docker compose up` |
|
|
||
| ### Code | ||
| - **Tests:** If you add a critical feature, please try to include a unit test. | ||
| - **Use of AI:** Add a .md file related to your implementation to allow ai tools to give better results for further implementation. |
There was a problem hiding this comment.
This guidance is unclear and has inconsistent capitalization (“ai tools”). If the intent is to add design/implementation notes for future contributors, please clarify what file(s) should be added, where they should live, and use “AI” consistently.
| - **Use of AI:** Add a .md file related to your implementation to allow ai tools to give better results for further implementation. | |
| - **Use of AI:** If your change introduces non-obvious design or implementation details, add a Markdown (`.md`) note near the related code or in the closest relevant documentation file. Include the purpose of the change, key technical decisions, and any important constraints so future contributors and AI tools have accurate context. |
| const [show, setShow] = useState(false); | ||
|
|
||
| return ( | ||
| <> | ||
| {/* Bulle flottante */} | ||
| <div | ||
| style={{ | ||
| position: "fixed", | ||
| bottom: "36px", | ||
| left: "32px", | ||
| zIndex: 9999, | ||
| }} | ||
| > | ||
| <button | ||
| onClick={() => setShow((v) => !v)} | ||
| className="bg-blue-600 text-white rounded-full w-12 h-12 flex items-center justify-center text-2xl shadow-lg hover:bg-blue-700 transition" | ||
| aria-label="Aide sur les contrôles 3D" | ||
| > | ||
| ? | ||
| </button> | ||
| </div> | ||
|
|
||
| {/* Fenêtre d'aide */} | ||
| <div className="fixed bottom-6 left-1/2 -translate-x-1/2 z-50"> | ||
| {show && ( | ||
| <div | ||
| style={{ | ||
| position: "fixed", | ||
| bottom: "92px", | ||
| left: "32px", | ||
| background: "white", | ||
| borderRadius: "16px", | ||
| boxShadow: "0 6px 32px rgba(0,0,0,0.18)", | ||
| padding: "24px", | ||
| minWidth: "260px", | ||
| maxWidth: "320px", | ||
| zIndex: 9999, | ||
| }} | ||
| > | ||
| <div className="flex items-center mb-3"> | ||
| <span className="text-blue-600 text-2xl font-bold mr-2">?</span> | ||
| <span className="font-semibold text-gray-900">Contrôles 3D</span> | ||
| <div className="absolute bottom-full mb-2 left-1/2 -translate-x-1/2 bg-surface-high/90 backdrop-blur-md border border-ghost-border/20 p-4 min-w-[260px] max-w-[320px]"> |
There was a problem hiding this comment.
show is initialized to false and there is no UI control to set it to true, so the tutorial popover can never be opened. Re-introduce an “open tutorial” button/trigger (or accept a prop/context to open it) so users can access the help.
| <span className="material-symbols-outlined text-mint text-base">help_outline</span> | ||
| <span className="text-xs font-bold text-on-surface-variant uppercase tracking-widest">Contrôles 3D</span> | ||
| <button | ||
| onClick={() => setShow(false)} | ||
| className="ml-auto px-2 py-1 text-sm text-gray-500 hover:text-red-500" | ||
| className="ml-auto p-1 text-on-surface-variant hover:text-on-surface transition-colors" | ||
| aria-label="Fermer l'aide" | ||
| > |
There was a problem hiding this comment.
The tutorial header and close aria-label are hardcoded French strings (e.g. “Contrôles 3D”, “Fermer l'aide”). Per repo guidelines, user-facing strings should come from useTranslation() and locale JSON keys.
| <div className="flex w-fit flex-col gap-1 self-start rounded-xl bg-surface-low/90 p-1 shadow-[0_8px_32px_0_rgba(0,0,0,0.5)] backdrop-blur-md"> | ||
| {transformTools.map((tool) => ( | ||
| <div key={tool.id} className="group relative"> | ||
| <div className="w-10 h-10 flex shrink-0 items-center justify-center rounded-lg text-on-surface-variant cursor-pointer"> |
There was a problem hiding this comment.
These tool icons are styled as interactive (cursor-pointer) but aren’t focusable or clickable (no onClick, not a <button>). If they’re meant to be actionable, render them as buttons with handlers + keyboard focus styles; otherwise remove the pointer cursor to avoid misleading UX.
| <div className="w-10 h-10 flex shrink-0 items-center justify-center rounded-lg text-on-surface-variant cursor-pointer"> | |
| <div className="w-10 h-10 flex shrink-0 items-center justify-center rounded-lg text-on-surface-variant"> |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.