Rework login UI #300
Rework login UI #300
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@copilot lint this |
… rework-login-form
vercel
bot
temporarily deployed
to
Preview – frontpage-oauth-preview-client
vercel
bot
temporarily deployed
to
Preview – frontpage-oauth-preview-client
vercel
bot
temporarily deployed
to
Preview – frontpage-oauth-preview-client
vercel
bot
temporarily deployed
to
Preview – frontpage-oauth-preview-client
vercel
bot
temporarily deployed
to
Preview – frontpage-oauth-preview-client
vercel
bot
temporarily deployed
to
Preview – frontpage-oauth-preview-client
vercel
bot
temporarily deployed
to
Preview – frontpage-oauth-preview-client
vercel
bot
temporarily deployed
to
Preview – frontpage-oauth-preview-client
… rework-login-form
There was a problem hiding this comment.
Pull request overview
This PR reworks the login UI to prioritize handle-based authentication with PDS URL login as a secondary option. The changes improve the user experience by making internet handle login the primary method while providing helpful educational content through accordion components.
Key changes:
- Restructured login flow to present handle-first authentication with PDS URL as secondary option
- Added new reusable UI components (Field, Accordion) for better form structure and help documentation
- Enhanced error handling with specific error messages for different failure scenarios
- Added @tailwindcss/typography plugin for better prose styling in help text
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added @tailwindcss/typography dependency and postcss-selector-parser as a transitive dependency |
| packages/frontpage/package.json | Added @tailwindcss/typography as dev dependency |
| packages/frontpage/tailwind.config.ts | Registered typography plugin in Tailwind configuration |
| packages/frontpage/lib/components/ui/label.tsx | Simplified component by removing unused cva variants and inline styling |
| packages/frontpage/lib/components/ui/field.tsx | New comprehensive field component system with multiple sub-components for flexible form layouts |
| packages/frontpage/lib/components/ui/accordion.tsx | New accordion component using Radix UI primitives for collapsible help sections |
| packages/frontpage/lib/auth-sign-in.ts | Added ErrorReason type export for typed error handling in UI |
| packages/frontpage/app/globals.css | Added custom prose styling for links with theme-aware colors |
| packages/frontpage/app/(auth)/login/_lib/form.tsx | Major restructure: handle-first login, added accordion help sections, improved error messages with specific handling per error type |
| packages/frontpage/app/(auth)/login/_lib/action.tsx | Simplified identifier error handling to return raw error code instead of formatted message |
| packages/frontpage/app/(auth)/layout.tsx | Changed layout from centered to top-aligned with vertical padding |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| That's an internet handle. | ||
| </p> | ||
| <p> | ||
| If you don't have one, choose "Continue with your |
There was a problem hiding this comment.
The help text references "Continue with your PDS" but the actual button text is "Sign up with a PDS". These should match for consistency and to avoid user confusion.
| If you don't have one, choose "Continue with your | |
| If you don't have one, choose "Sign up with a |
| target="_blank" | ||
| rel="noreferrer" | ||
| > | ||
| internethandle.org <OpenInNewWindowIcon className="inline" /> |
There was a problem hiding this comment.
The link should include a screen reader accessible label indicating it opens in a new window. Consider adding aria-label="internethandle.org (opens in new window)" or using visually hidden text instead of just the icon to improve accessibility for screen reader users.
| internethandle.org <OpenInNewWindowIcon className="inline" /> | |
| internethandle.org{" "} | |
| <OpenInNewWindowIcon className="inline" /> | |
| <span className="sr-only">(opens in new window)</span> |
| id="identifier" | ||
| name="identifier" | ||
| required | ||
| placeholder="eg. dril.bsky.social" |
There was a problem hiding this comment.
The input field lacks an associated label element. While it has a placeholder, this doesn't provide adequate accessibility for screen reader users. Consider adding a Label component or aria-label attribute to improve accessibility.
| placeholder="eg. dril.bsky.social" | |
| placeholder="eg. dril.bsky.social" | |
| aria-label="Internet handle" |
| <Input | ||
| name="pdsUrl" | ||
| placeholder="eg. bsky.social" | ||
| defaultValue={DEFAULT_PDS.host} |
There was a problem hiding this comment.
The input field lacks an associated label element. While it has a placeholder, this doesn't provide adequate accessibility for screen reader users. Consider adding a Label component or aria-label attribute to improve accessibility.
| defaultValue={DEFAULT_PDS.host} | |
| defaultValue={DEFAULT_PDS.host} | |
| aria-label="Personal data server host" |
Rework login to be handle-first with a secondary PDS URL based login method.