Add live artifact editor and demo login#2
Open
DhruvalAnandkar wants to merge 1 commit into
Open
Conversation
- New ArtifactEditor with Preview/Split/Edit modes, live markdown preview, editable title, dirty-state tracking, Ctrl/Cmd+S save, Revert, and .docx export - Wire ArtifactEditor into ProjectWorkspace, replacing the read-only viewer - Add POST /api/auth/dev-login endpoint (disabled in production) that creates or reuses a demo user and issues a standard session token - Add DevLoginRequest schema and authApi.devLogin client - Add 'Try the Demo (no sign-up)' button on LoginPage with loading and error states
📝 WalkthroughWalkthroughAdded a development-only login endpoint ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant AuthAPI as Auth API
participant DB as Database
Client->>AuthAPI: POST /dev-login { email?, name? }
alt is_production == true
AuthAPI-->>Client: HTTP 404 Not Found
else
AuthAPI->>DB: Query User by google_id: "demo:{email}"
alt User exists
DB-->>AuthAPI: Existing User
else
AuthAPI->>DB: Create new User with google_id, email, name
DB-->>AuthAPI: Created User
end
AuthAPI->>DB: Update User name (if changed)
AuthAPI->>DB: Create session token for user.id
DB-->>AuthAPI: Session token
AuthAPI-->>Client: { user, session_token }, Set-Cookie: session_token
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
frontend/src/components/ArtifactEditor.tsx (2)
70-95: Minor: redundant state writes after save, and consider guarding against stale saves.After
artifactsApi.updatereturns, you setsavedTitle/savedContentand then alsosetTitle(data.title)/setContent(data.content_markdown). If the server normalizes content (e.g., trims trailing whitespace), this will silently overwrite any edits the user made while the request was in flight — the UI allows continued typing duringsavingonly if they bypass the disabled button, but Ctrl+S is still active and the textarea is not disabled during save. Consider either disabling input whilesaving, or only syncing thesaved*refs and leaving the livetitle/contentalone when they've diverged from the pre-save snapshot. Not a blocker for the happy path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ArtifactEditor.tsx` around lines 70 - 95, The save handler (handleSave) is overwriting the live title/content after artifactsApi.update returns, which can clobber edits made while the request was in flight; either disable editing while saving by using the saving state to disable the inputs, or change the post-save sync to only update savedTitle/savedContent (and other saved refs like setVersion/setUpdatedAt) and avoid calling setTitle/setContent unless the current title/content still equal the pre-save snapshot (savedTitle/savedContent) to prevent stomping concurrent edits; implement the chosen approach within handleSave and the input components, referencing savedTitle, savedContent, setTitle, setContent, and saving.
117-126: Keyboard listener rebinds on every render; prefer a ref oruseCallback.
handleSaveis a new function identity on every render, so this effect detaches and reattaches the globalkeydownlistener on every keystroke in the textarea. Functionally fine, but wasteful and easy to fix by either reading the latest handler via a ref, or wrappinghandleSaveinuseCallbackwith stable deps. An empty-deps effect with a ref also avoids any subtle stale-closure questions.♻️ Suggested refactor
+ const handleSaveRef = useRef(handleSave); + useEffect(() => { handleSaveRef.current = handleSave; }); useEffect(() => { const onKey = (e: KeyboardEvent) => { if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 's') { e.preventDefault(); - void handleSave(); + void handleSaveRef.current(); } }; window.addEventListener('keydown', onKey); return () => window.removeEventListener('keydown', onKey); - }, [handleSave]); + }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/ArtifactEditor.tsx` around lines 117 - 126, The effect is reattaching the global keydown listener because handleSave has a new identity each render; change this by making the handler stable: either wrap handleSave in useCallback with the correct dependency list and keep the effect deps as [handleSave], or keep handleSave as-is and create a ref (e.g., saveHandlerRef) that you update on each render and use an empty-deps useEffect to add/remove the onKey listener which calls saveHandlerRef.current(); update references to onKey/window.addEventListener/window.removeEventListener accordingly so the listener is not rebound on every render.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/auth.py`:
- Around line 154-170: The current demo signup creates a User by google_id but
can violate the unique User.email constraint if an unauthenticated caller
supplies an email already owned by another account; before db.add() perform a
lookup by email (select(User).where(User.email == email)) and if a record exists
with a different google_id return a 400/validation error (or reject
non-demo-domain emails by checking the domain portion of request.email and
returning 400 for non-demo domains); only proceed to create and db.add() when no
conflicting email owner exists. Ensure you reference and compare the
existing_user.email and existing_user.google_id against the computed google_id
to detect collisions.
In `@backend/app/schemas/auth.py`:
- Around line 16-19: The DevLoginRequest schema allows arbitrarily long emails
which can overflow the DB String(255) columns (and the endpoint prefixes
google_id with "demo:"), so add length validation at the schema boundary: update
the DevLoginRequest.email declaration to enforce max_length=255 (e.g. email:
Optional[str] = Field(None, max_length=255) or use constr(max_length=255)),
import Field/constr from pydantic as needed, and keep the Optional typing; this
prevents oversized values before hitting User.email/User.google_id.
In `@frontend/src/components/ArtifactEditor.tsx`:
- Around line 103-114: The filename derived from the artifact title used in
handleExport can contain characters invalid for files (/, \, :, *, newlines,
etc.); update handleExport to sanitize/slugify title before calling
downloadBlob: inside handleExport (which calls artifactsApi.exportDocx and
downloadBlob), compute a safeName by trimming and replacing invalid filename
characters (e.g. remove or replace / \ : * ? " < > | and newlines, collapse
whitespace to single hyphen or underscore, and limit length), fall back to
'artifact' if result is empty, then pass `${safeName}.docx` to downloadBlob
instead of `${title || 'artifact'}.docx`.
- Around line 56-66: The useEffect that watches artifact.id unconditionally
calls setTitle, setContent, setSavedTitle, setSavedContent, setVersion,
setUpdatedAt and resets state (in ArtifactEditor) which discards unsaved edits;
update this to detect a dirty state (compare title/content vs
savedTitle/savedContent or expose an isDirty flag) and before overwriting either
prompt the user to confirm switching or call a parent callback like
onRequestClose/canSwitch to ask permission, and also guard the onClose handler
(onClick={onClose}) to show the same confirmation when there are unsaved
changes; alternatively implement the parent-controlled remount pattern
(key={artifact.id}) so the parent decides when to force reset.
- Around line 174-179: The mode switcher is hidden on small screens due to the
"hidden sm:flex" wrapper, preventing mobile users from switching out of preview;
update the UI so mobile can toggle modes (at minimum Preview <-> Edit). Modify
the container around ModeButton (or its parent) to be visible on small screens
(remove or adjust "hidden sm:flex") or render an alternate compact control for
small viewports that still calls setMode and reads mode (e.g., a two-option
toggle showing ModeButton or a simplified toggle when width < sm), ensuring
ModeButton, setMode, and mode are used so Edit/Split/Preview remain reachable on
mobile.
In `@frontend/src/pages/LoginPage.tsx`:
- Around line 149-167: The demo error message currently rendered from demoError
isn’t exposed to assistive tech; update the demo login button (onClick handler
handleDemoLogin) and the error element so failures are announced by screen
readers: give the error <p> a stable id (e.g., demo-error) and add role="alert"
or aria-live="assertive" (and aria-atomic="true") to that element, then add
aria-describedby="demo-error" on the demo button (and ensure demoError is only
present when there is a message) so the button is programmatically connected to
the error text for screen-reader users.
---
Nitpick comments:
In `@frontend/src/components/ArtifactEditor.tsx`:
- Around line 70-95: The save handler (handleSave) is overwriting the live
title/content after artifactsApi.update returns, which can clobber edits made
while the request was in flight; either disable editing while saving by using
the saving state to disable the inputs, or change the post-save sync to only
update savedTitle/savedContent (and other saved refs like
setVersion/setUpdatedAt) and avoid calling setTitle/setContent unless the
current title/content still equal the pre-save snapshot
(savedTitle/savedContent) to prevent stomping concurrent edits; implement the
chosen approach within handleSave and the input components, referencing
savedTitle, savedContent, setTitle, setContent, and saving.
- Around line 117-126: The effect is reattaching the global keydown listener
because handleSave has a new identity each render; change this by making the
handler stable: either wrap handleSave in useCallback with the correct
dependency list and keep the effect deps as [handleSave], or keep handleSave
as-is and create a ref (e.g., saveHandlerRef) that you update on each render and
use an empty-deps useEffect to add/remove the onKey listener which calls
saveHandlerRef.current(); update references to
onKey/window.addEventListener/window.removeEventListener accordingly so the
listener is not rebound on every render.
🪄 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: 84b0f747-05b2-42ab-a33e-409dda977b6a
📒 Files selected for processing (7)
backend/app/api/auth.pybackend/app/schemas/__init__.pybackend/app/schemas/auth.pyfrontend/src/components/ArtifactEditor.tsxfrontend/src/lib/api.tsfrontend/src/pages/LoginPage.tsxfrontend/src/pages/ProjectWorkspace.tsx
Comment on lines
+154
to
+170
| email = (request.email or "demo@cortexlab.local").strip().lower() | ||
| name = (request.name or "Demo Researcher").strip() or "Demo Researcher" | ||
| # Stable synthetic google_id so the same demo email always maps to the same user | ||
| google_id = f"demo:{email}" | ||
|
|
||
| result = await db.execute(select(User).where(User.google_id == google_id)) | ||
| user = result.scalar_one_or_none() | ||
|
|
||
| if not user: | ||
| user = User( | ||
| google_id=google_id, | ||
| email=email, | ||
| name=name, | ||
| avatar_url=None, | ||
| ) | ||
| db.add(user) | ||
| await db.commit() |
There was a problem hiding this comment.
Handle demo email collisions before inserting.
The route looks up only by google_id, then inserts email. If an unauthenticated caller supplies an email already used by another account, the unique User.email constraint will fail and return a 500. Reject non-demo-domain emails or explicitly detect collisions before db.add().
🛡️ Proposed collision guard
email = (request.email or "demo@cortexlab.local").strip().lower()
name = (request.name or "Demo Researcher").strip() or "Demo Researcher"
+
+ if not email.endswith("@cortexlab.local"):
+ raise HTTPException(status_code=400, detail="Demo login requires a demo email address")
+
# Stable synthetic google_id so the same demo email always maps to the same user
google_id = f"demo:{email}"
result = await db.execute(select(User).where(User.google_id == google_id))
user = result.scalar_one_or_none()
if not user:
+ existing_email_result = await db.execute(select(User).where(User.email == email))
+ existing_email_user = existing_email_result.scalar_one_or_none()
+ if existing_email_user:
+ raise HTTPException(status_code=409, detail="Demo email is already in use")
+
user = User(
google_id=google_id,
email=email,
name=name,
avatar_url=None,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| email = (request.email or "demo@cortexlab.local").strip().lower() | |
| name = (request.name or "Demo Researcher").strip() or "Demo Researcher" | |
| # Stable synthetic google_id so the same demo email always maps to the same user | |
| google_id = f"demo:{email}" | |
| result = await db.execute(select(User).where(User.google_id == google_id)) | |
| user = result.scalar_one_or_none() | |
| if not user: | |
| user = User( | |
| google_id=google_id, | |
| email=email, | |
| name=name, | |
| avatar_url=None, | |
| ) | |
| db.add(user) | |
| await db.commit() | |
| email = (request.email or "demo@cortexlab.local").strip().lower() | |
| name = (request.name or "Demo Researcher").strip() or "Demo Researcher" | |
| if not email.endswith("@cortexlab.local"): | |
| raise HTTPException(status_code=400, detail="Demo login requires a demo email address") | |
| # Stable synthetic google_id so the same demo email always maps to the same user | |
| google_id = f"demo:{email}" | |
| result = await db.execute(select(User).where(User.google_id == google_id)) | |
| user = result.scalar_one_or_none() | |
| if not user: | |
| existing_email_result = await db.execute(select(User).where(User.email == email)) | |
| existing_email_user = existing_email_result.scalar_one_or_none() | |
| if existing_email_user: | |
| raise HTTPException(status_code=409, detail="Demo email is already in use") | |
| user = User( | |
| google_id=google_id, | |
| email=email, | |
| name=name, | |
| avatar_url=None, | |
| ) | |
| db.add(user) | |
| await db.commit() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/api/auth.py` around lines 154 - 170, The current demo signup
creates a User by google_id but can violate the unique User.email constraint if
an unauthenticated caller supplies an email already owned by another account;
before db.add() perform a lookup by email (select(User).where(User.email ==
email)) and if a record exists with a different google_id return a
400/validation error (or reject non-demo-domain emails by checking the domain
portion of request.email and returning 400 for non-demo domains); only proceed
to create and db.add() when no conflicting email owner exists. Ensure you
reference and compare the existing_user.email and existing_user.google_id
against the computed google_id to detect collisions.
Comment on lines
+16
to
+19
| class DevLoginRequest(BaseModel): | ||
| """Request body for dummy/demo login (dev & demos only).""" | ||
| name: Optional[str] = None | ||
| email: Optional[str] = None |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect declared Pydantic version so the validation API can be matched to the installed major version.
fd -i '^(pyproject\.toml|requirements.*\.txt|poetry\.lock|Pipfile)$' \
-x sh -c 'echo "--- $1"; rg -n "pydantic|email-validator" "$1" || true' sh {}Repository: Aniket25042003/CortexLab
Length of output: 144
🏁 Script executed:
rg -n "email-validator" --type txt --type tomlRepository: Aniket25042003/CortexLab
Length of output: 50
🏁 Script executed:
fd -i 'user.py' -path '*/models/*' -x cat -n {}Repository: Aniket25042003/CortexLab
Length of output: 236
🏁 Script executed:
rg -n "demo:" --type pyRepository: Aniket25042003/CortexLab
Length of output: 127
🏁 Script executed:
rg -n "DevLoginRequest" --type py -A 5Repository: Aniket25042003/CortexLab
Length of output: 1757
🏁 Script executed:
rg -n "email-validator" backend/Repository: Aniket25042003/CortexLab
Length of output: 50
🏁 Script executed:
fd -i 'user.py' -path '*/models/*' -x head -100 {}Repository: Aniket25042003/CortexLab
Length of output: 236
🏁 Script executed:
sed -n '140,170p' backend/app/api/auth.pyRepository: Aniket25042003/CortexLab
Length of output: 1117
🏁 Script executed:
find backend -name "*.py" -path "*/models/*" | head -5Repository: Aniket25042003/CortexLab
Length of output: 231
🏁 Script executed:
rg -n "class User" backend/ --type py -A 10Repository: Aniket25042003/CortexLab
Length of output: 1230
🏁 Script executed:
cat backend/requirements.txtRepository: Aniket25042003/CortexLab
Length of output: 753
🏁 Script executed:
cat -n backend/app/models/user.pyRepository: Aniket25042003/CortexLab
Length of output: 1698
Validate demo login input at the schema boundary.
email can currently be any length, but User.email and User.google_id are both String(255). Since the endpoint prefixes google_id with demo:, an oversized email would overflow the column. Add max_length validation to prevent DB errors.
🛡️ Proposed validation
-from pydantic import BaseModel
+from pydantic import BaseModel, Field
from typing import Optional
class DevLoginRequest(BaseModel):
"""Request body for dummy/demo login (dev & demos only)."""
- name: Optional[str] = None
- email: Optional[str] = None
+ name: Optional[str] = Field(default=None, max_length=255)
+ email: Optional[str] = Field(default=None, max_length=250)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/schemas/auth.py` around lines 16 - 19, The DevLoginRequest schema
allows arbitrarily long emails which can overflow the DB String(255) columns
(and the endpoint prefixes google_id with "demo:"), so add length validation at
the schema boundary: update the DevLoginRequest.email declaration to enforce
max_length=255 (e.g. email: Optional[str] = Field(None, max_length=255) or use
constr(max_length=255)), import Field/constr from pydantic as needed, and keep
the Optional typing; this prevents oversized values before hitting
User.email/User.google_id.
Comment on lines
+56
to
+66
| // When the selected artifact changes, reset all state | ||
| useEffect(() => { | ||
| setTitle(artifact.title); | ||
| setContent(artifact.content_markdown); | ||
| setSavedTitle(artifact.title); | ||
| setSavedContent(artifact.content_markdown); | ||
| setVersion(artifact.version); | ||
| setUpdatedAt(artifact.updated_at); | ||
| setMode('preview'); | ||
| setError(null); | ||
| }, [artifact.id]); |
There was a problem hiding this comment.
Silent data loss: switching artifacts (or receiving a prop update) discards unsaved edits.
The reset effect keys on artifact.id and overwrites title/content unconditionally. If the user has unsaved edits and clicks a different artifact in the sidebar (or ProjectWorkspace replaces the prop after a background refetch), their work disappears with no warning.
Consider guarding with an isDirty confirmation, or at minimum prompting before overwriting:
🛡️ Proposed guard
useEffect(() => {
+ if (title !== savedTitle || content !== savedContent) {
+ const keep = window.confirm(
+ 'You have unsaved changes in the current artifact. Discard them and switch?'
+ );
+ if (!keep) {
+ // Caller should re-select the previous artifact; surfacing this via a callback
+ // would be cleaner than confirm(), but at minimum avoid silently dropping edits.
+ }
+ }
setTitle(artifact.title);
setContent(artifact.content_markdown);
...
}, [artifact.id]);A cleaner pattern is to lift the dirty check to ProjectWorkspace via an onRequestClose/canSwitch callback, or to key the whole <ArtifactEditor key={artifact.id} /> and let the parent decide when to remount.
The same concern applies to the X close button at line 216 (onClick={onClose}) — closing with unsaved changes silently drops them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ArtifactEditor.tsx` around lines 56 - 66, The
useEffect that watches artifact.id unconditionally calls setTitle, setContent,
setSavedTitle, setSavedContent, setVersion, setUpdatedAt and resets state (in
ArtifactEditor) which discards unsaved edits; update this to detect a dirty
state (compare title/content vs savedTitle/savedContent or expose an isDirty
flag) and before overwriting either prompt the user to confirm switching or call
a parent callback like onRequestClose/canSwitch to ask permission, and also
guard the onClose handler (onClick={onClose}) to show the same confirmation when
there are unsaved changes; alternatively implement the parent-controlled remount
pattern (key={artifact.id}) so the parent decides when to force reset.
Comment on lines
+103
to
+114
| const handleExport = async () => { | ||
| setExporting(true); | ||
| try { | ||
| const response = await artifactsApi.exportDocx(artifact.id); | ||
| downloadBlob(response.data, `${title || 'artifact'}.docx`); | ||
| } catch (err) { | ||
| console.error('Export failed:', err); | ||
| setError('Export failed.'); | ||
| } finally { | ||
| setExporting(false); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Sanitize the filename derived from the artifact title.
title can contain /, \, :, *, newlines, etc., which produce broken/rejected downloads across OSes. Run the title through a simple slug before passing to downloadBlob.
♻️ Proposed fix
- downloadBlob(response.data, `${title || 'artifact'}.docx`);
+ const safeName = (title || 'artifact')
+ .replace(/[\\/:*?"<>|\r\n]+/g, '_')
+ .replace(/\s+/g, ' ')
+ .trim()
+ .slice(0, 120) || 'artifact';
+ downloadBlob(response.data, `${safeName}.docx`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ArtifactEditor.tsx` around lines 103 - 114, The
filename derived from the artifact title used in handleExport can contain
characters invalid for files (/, \, :, *, newlines, etc.); update handleExport
to sanitize/slugify title before calling downloadBlob: inside handleExport
(which calls artifactsApi.exportDocx and downloadBlob), compute a safeName by
trimming and replacing invalid filename characters (e.g. remove or replace / \ :
* ? " < > | and newlines, collapse whitespace to single hyphen or underscore,
and limit length), fall back to 'artifact' if result is empty, then pass
`${safeName}.docx` to downloadBlob instead of `${title || 'artifact'}.docx`.
Comment on lines
+174
to
+179
| {/* Mode switcher */} | ||
| <div className="hidden sm:flex items-center p-1 bg-slate-100 rounded-xl"> | ||
| <ModeButton active={mode === 'preview'} onClick={() => setMode('preview')} icon={<Eye className="w-3.5 h-3.5" />} label="Preview" /> | ||
| <ModeButton active={mode === 'split'} onClick={() => setMode('split')} icon={<Columns2 className="w-3.5 h-3.5" />} label="Split" /> | ||
| <ModeButton active={mode === 'edit'} onClick={() => setMode('edit')} icon={<Pencil className="w-3.5 h-3.5" />} label="Edit" /> | ||
| </div> |
There was a problem hiding this comment.
Mode switcher hidden on small screens leaves mobile users stuck in Preview.
hidden sm:flex means below the sm breakpoint the Edit/Split/Preview buttons never render, so mobile/narrow viewports cannot enter edit mode at all — the title input, textarea, Save, and Revert UI are effectively unreachable. If mobile editing isn't a goal, the Save/Revert/Export buttons could also be simplified for that viewport; if it is, expose at least a toggle between Preview and Edit on small screens.
♻️ Suggestion
- <div className="hidden sm:flex items-center p-1 bg-slate-100 rounded-xl">
+ <div className="flex items-center p-1 bg-slate-100 rounded-xl">
<ModeButton active={mode === 'preview'} ... label="Preview" />
- <ModeButton active={mode === 'split'} ... label="Split" />
+ <ModeButton active={mode === 'split'} ... label="Split" className="hidden sm:flex" />
<ModeButton active={mode === 'edit'} ... label="Edit" />
</div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/ArtifactEditor.tsx` around lines 174 - 179, The mode
switcher is hidden on small screens due to the "hidden sm:flex" wrapper,
preventing mobile users from switching out of preview; update the UI so mobile
can toggle modes (at minimum Preview <-> Edit). Modify the container around
ModeButton (or its parent) to be visible on small screens (remove or adjust
"hidden sm:flex") or render an alternate compact control for small viewports
that still calls setMode and reads mode (e.g., a two-option toggle showing
ModeButton or a simplified toggle when width < sm), ensuring ModeButton,
setMode, and mode are used so Edit/Split/Preview remain reachable on mobile.
Comment on lines
+149
to
+167
| <button | ||
| onClick={handleDemoLogin} | ||
| disabled={demoLoading} | ||
| className="w-full flex items-center gap-3 px-6 py-4 bg-gradient-to-r from-indigo-600 to-violet-600 text-white rounded-2xl font-semibold text-sm hover:shadow-lg hover:shadow-indigo-500/30 transition-all shadow-sm active:scale-[0.99] group disabled:opacity-70 disabled:cursor-not-allowed" | ||
| > | ||
| {demoLoading ? ( | ||
| <Loader2 className="w-5 h-5 animate-spin" /> | ||
| ) : ( | ||
| <Zap className="w-5 h-5" /> | ||
| )} | ||
| <span className="flex-1 text-center"> | ||
| {demoLoading ? 'Preparing demo workspace...' : 'Try the Demo (no sign-up)'} | ||
| </span> | ||
| <ArrowRight className="w-4 h-4 text-white/80 group-hover:translate-x-0.5 transition-all" /> | ||
| </button> | ||
|
|
||
| {demoError && ( | ||
| <p className="text-xs text-red-500 text-center font-medium">{demoError}</p> | ||
| )} |
There was a problem hiding this comment.
Make demo login failures announceable.
demoError appears after an async action; add role="alert"/aria-live and connect it to the button so screen-reader users receive the failure state.
♿ Proposed accessibility fix
<button
onClick={handleDemoLogin}
disabled={demoLoading}
+ aria-busy={demoLoading}
+ aria-describedby={demoError ? 'demo-login-error' : undefined}
className="w-full flex items-center gap-3 px-6 py-4 bg-gradient-to-r from-indigo-600 to-violet-600 text-white rounded-2xl font-semibold text-sm hover:shadow-lg hover:shadow-indigo-500/30 transition-all shadow-sm active:scale-[0.99] group disabled:opacity-70 disabled:cursor-not-allowed"
>
@@
{demoError && (
- <p className="text-xs text-red-500 text-center font-medium">{demoError}</p>
+ <p
+ id="demo-login-error"
+ role="alert"
+ aria-live="polite"
+ className="text-xs text-red-500 text-center font-medium"
+ >
+ {demoError}
+ </p>
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/pages/LoginPage.tsx` around lines 149 - 167, The demo error
message currently rendered from demoError isn’t exposed to assistive tech;
update the demo login button (onClick handler handleDemoLogin) and the error
element so failures are announced by screen readers: give the error <p> a stable
id (e.g., demo-error) and add role="alert" or aria-live="assertive" (and
aria-atomic="true") to that element, then add aria-describedby="demo-error" on
the demo button (and ensure demoError is only present when there is a message)
so the button is programmatically connected to the error text for screen-reader
users.
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 by CodeRabbit