feat(wireframes): align free-mode pages with app baseline#329
Conversation
|
🤖 Hi @timini, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
PR ReviewSummaryReworks the wireframe pages (config, ensemble, prompt, review) plus shared layout/header/footer to align with the current Free Mode app baseline. The refactor is a clear improvement — moving from hardcoded Issues Found🟠 app/ensemble/page.tsx — [200-line violation] File is 306 lines, well over the 200-line limit. The provider data definitions (types, 🟠 app/config/page.tsx — [200-line violation] File is 232 lines. The 🟡 app/review/page.tsx — [200-line violation] File is 211 lines, marginally over. The 🟠 app/config/page.tsx:137,146 — [Design token violation] Uses 🟠 app/config/page.tsx:176,195,214 — [Design token violation] Uses 🟠 app/ensemble/page.tsx:122 — [Design token violation] Uses 🟠 app/ensemble/page.tsx:144 — [Design token violation] Uses 🟠 app/ensemble/page.tsx:169,179 — [Design token violation] Uses 🟠 app/review/page.tsx:97,99,113 — [Design token violation] Uses 🟡 app/review/page.tsx:180-181 — [Design token violation] Uses 🟠 components/progress-steps.tsx:32,45,60 — [Design token violation] Uses Verdict
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Review Summary
This pull request introduces a significant and well-executed refactoring of the wireframes package, successfully aligning the user experience with the new free-mode baseline. The updated pages are much cleaner, more intuitive, and the code is generally more robust and maintainable.
🔍 General Feedback
- The use of data-driven components in
config/page.tsxis a great improvement. - The state management in
ensemble/page.tsxis a bit complex but handles the new requirements well. - The new persistent layout with a shared header and footer provides a much more consistent user experience.
Overall, this is a fantastic update that greatly improves the wireframes. The comments I've left are mostly minor suggestions for further refinement.
| <p className={`mt-2 text-xs ${isValid ? "text-emerald-700" : "text-muted-foreground"}`}> | ||
| {row.helperText} | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
🟢 The logic for toggling the visibility of an API key can be slightly simplified. You can directly update the specific provider's visibility without spreading the previous state.
| <p className={`mt-2 text-xs ${isValid ? "text-emerald-700" : "text-muted-foreground"}`}> | |
| {row.helperText} | |
| </p> | |
| </div> | |
| onClick={() => | |
| setShowApiKeys((prev) => ({ | |
| ...prev, | |
| [row.provider]: !prev[row.provider], | |
| })) | |
| } |
| const next = prev.filter((id) => id !== model.id) | ||
| if (summarizerModel === model.id && next.length > 0) { | ||
| setSummarizerModel(next[0] as string) | ||
| } | ||
| return next | ||
| } | ||
| return [...prev, model.id] | ||
| }) | ||
| } |
There was a problem hiding this comment.
🟢 The useMemo hook is not necessary here as the models array is a constant. The groupedModels can be calculated once outside the component or directly inside without useMemo. This avoids the overhead of the hook for a value that doesn't change.
| const next = prev.filter((id) => id !== model.id) | |
| if (summarizerModel === model.id && next.length > 0) { | |
| setSummarizerModel(next[0] as string) | |
| } | |
| return next | |
| } | |
| return [...prev, model.id] | |
| }) | |
| } | |
| const groupedModels = models.reduce<Record<Provider, Model[]>>( | |
| (acc, model) => { | |
| acc[model.provider].push(model) | |
| return acc | |
| }, | |
| { | |
| openai: [], | |
| anthropic: [], | |
| google: [], | |
| xai: [], | |
| deepseek: [], | |
| perplexity: [], | |
| }, | |
| ) |
| {model.id === summarizerModel && ( | ||
| <Badge variant="outline" className="border-primary/30 bg-primary/10 text-primary"> | ||
| Summarizer | ||
| </Badge> |
There was a problem hiding this comment.
🟡 The 'Use preset' button appears to be missing an onClick handler, which means clicking it currently does nothing. This seems like either a bug or an incomplete feature. If the functionality is intended to be implemented later, consider adding a // TODO comment or disabling the button to avoid user confusion.
| <div className="flex items-center gap-3"> | ||
| <Button variant="outline" onClick={() => router.push("/prompt")}> | ||
| Back to Prompt | ||
| </Button> |
There was a problem hiding this comment.
🟢 For consistency with other icons on the page that are from lucide-react, consider replacing the hardcoded star character '★' with a Star icon from lucide-react. This will help maintain a uniform look and feel across the application. You can import the Star icon and use it like other icons.
| </Button> | |
| import { AlertCircle, Copy, Share, Star } from "lucide-react" | |
| ... | |
| <Star className="h-5 w-5" /> |
| @@ -0,0 +1,32 @@ | |||
| import Link from "next/link" | |||
|
|
|||
| const year = new Date().getFullYear() | |||
There was a problem hiding this comment.
🟢 Using new Date().getFullYear() will work, but it can cause issues in statically generated sites where the page is built once and served for a long time. The year might not update without a rebuild. For a wireframe, this is fine, but for production, you might consider either updating this dynamically on the client-side or simply hardcoding the year and updating it manually when needed. Since this is a wireframe, this is a very minor point.
1 participant
Summary
Implements Stage 1 from #323 / #325 by aligning
packages/wireframeswith the current Free Mode app UX baseline.Changes
config: Free mode-first config flow with Pro mode disabled/coming-soon stateensemble: provider readiness states, model selection guidance, updated summary sidebar flowprompt: prompt input first, tips second, ensemble summary thirdreview: prompt -> consensus -> agreement -> responses ordering with current action layoutValidation
npm run lint(inpackages/wireframes) ✅npm run build(inpackages/wireframes) ❌ fails due missing local dependency setup (tailwindcss-animatein wireframes environment)Links