feat(ui/portal): create singin page components #276
Conversation
Note: not hooked up with BE
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive set of changes to the portal application, focusing on implementing a sign-in page and enhancing the project's configuration. The changes include creating new components for the sign-in flow, updating Storybook configuration, modifying the index route, and adding TypeScript configuration files for the portal library. The modifications aim to improve the application's structure, add a new sign-in experience, and prepare the project for further development. Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for cuhacking-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
libs/portal/features/sign-in/ui/call-to-action.stories.tsx (1)
4-16: Enhance Storybook coverage with additional variants.The story configuration looks good, but consider adding variants for different states:
- Loading state
- Error state
- Disabled state
Example addition:
export const Loading: Story = { args: { isLoading: true } } export const Error: Story = { args: { error: 'Failed to authenticate' } }libs/portal/pages/signin/index.tsx (2)
10-15: Optimize background image loading and accessibility.The background image implementation could be improved:
- Add loading="lazy" for performance
- Consider using next/image for automatic optimization
- Provide more descriptive alt text even if decorative
<img src={dashboard_background} - alt="" + alt="Abstract dashboard background" + loading="lazy" aria-hidden="true" className="absolute top-0 left-0 w-full h-full object-cover z-[-1]" />
4-18: Add SEO metadata for the signin page.Consider adding a title and meta description for better SEO:
import { Helmet } from 'react-helmet'; export function Signin() { return ( <> <Helmet> <title>Sign In - cuHacking Portal</title> <meta name="description" content="Sign in to cuHacking portal using your GitHub account" /> </Helmet> {/* existing JSX */} </> ); }libs/portal/features/sign-in/ui/call-to-action.tsx (1)
5-31: Add TypeScript props interface for better reusability.Consider adding props to make the component more flexible:
interface SignInCallToActionProps { onLogin?: () => Promise<void>; isLoading?: boolean; error?: string; } export function SignInCallToAction({ onLogin, isLoading = false, error }: SignInCallToActionProps) { // ... existing implementation }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
libs/shared/assets/icons/socials/github-black-1.svgis excluded by!**/*.svg
📒 Files selected for processing (11)
.storybook/main.ts(2 hunks)apps/portal/app/routes/_index.tsx(1 hunks)libs/portal/features/sign-in/index.ts(1 hunks)libs/portal/features/sign-in/ui/call-to-action.stories.tsx(1 hunks)libs/portal/features/sign-in/ui/call-to-action.tsx(1 hunks)libs/portal/pages/signin/index.tsx(1 hunks)libs/portal/project.json(1 hunks)libs/portal/tsconfig.json(1 hunks)libs/portal/tsconfig.lib.json(1 hunks)libs/shared/ui/button/button.tsx(1 hunks)tsconfig.base.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- libs/portal/project.json
- libs/portal/features/sign-in/index.ts
- libs/portal/tsconfig.json
- libs/portal/tsconfig.lib.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (7)
apps/portal/app/routes/_index.tsx (1)
2-9:⚠️ Potential issueVerify the removal of dynamic social links loading.
The change from dynamically loaded social links to a static array seems like a potential regression. Was this intentional? If so, please provide context for this change in the PR description.
libs/portal/features/sign-in/ui/call-to-action.tsx (1)
5-31: Add unit tests for the SignInCallToAction component.As mentioned in the PR objectives, tests are missing. Please add unit tests to cover:
- Rendering in different states
- Button click handling
- Loading state
- Error handling
Would you like me to help generate the test file with these test cases?
libs/shared/ui/button/button.tsx (2)
Line range hint
6-6: Address TODO comment before merging.The TODO comment suggests a pending refactor. Since you're modifying the button component, consider addressing this technical debt now.
Would you like me to help create a separate issue to track this refactor task?
22-22: LGTM! The primary variant follows consistent styling patterns.The new primary variant maintains consistency with existing variants and uses appropriate Tailwind classes.
.storybook/main.ts (2)
18-18: LGTM! Story path addition aligns with new portal features.The new path correctly includes stories from the portal features directory, which is necessary for the signin page components.
115-117: LGTM! Essential fix for webpack configuration.Adding the missing return statement ensures proper webpack configuration.
tsconfig.base.json (1)
20-21: LGTM! Path mappings follow established patterns.The new portal module path mappings maintain consistency with existing module configurations and enable proper TypeScript resolution for the signin page components.
| <Button | ||
| variant="primary" | ||
| className="flex items-center gap-x-3 px-4" | ||
| aria-label="Login with GitHub" | ||
| > | ||
| <img src={github_black} alt="GitHub logo" className="h-5 w-5" /> | ||
| <span>Login</span> | ||
| </Button> |
There was a problem hiding this comment.
Add onClick handler and loading state to the login button.
The button is missing crucial functionality:
- onClick handler for GitHub authentication
- Loading state feedback
- Error handling
<Button
variant="primary"
className="flex items-center gap-x-3 px-4"
aria-label="Login with GitHub"
+ onClick={handleGitHubLogin}
+ isLoading={isLoading}
+ disabled={isLoading}
>
- <img src={github_black} alt="GitHub logo" className="h-5 w-5" />
+ {!isLoading && <img src={github_black} alt="GitHub logo" className="h-5 w-5" />}
<span>Login</span>
</Button>Committable suggestion skipped: line range outside the PR's diff.
|
🎉 This PR is included in version 1.23.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Checklist
Summary by CodeRabbit
Release Notes
New Features
UI Improvements
Development
Chores