Improve Contributors Button UI and Landing Page Visual Polish - #128
Improve Contributors Button UI and Landing Page Visual Polish#128broskell wants to merge 3 commits into
Conversation
|
@broskell is attempting to deploy a commit to the Sahil's projects Team on Vercel. A member of the Team first needs to authorize it. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on enhancing the visual appeal and user experience of the application's landing page, particularly by modernizing the 'Contributors' button and applying general UI polish. Concurrently, it refactors the Firebase utility file to improve robustness and simplify authentication logic, ensuring a more stable and maintainable codebase. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request successfully implements the visual styling improvements for the Contributors button and other landing page elements as described. The new CSS classes are well-defined and applied correctly in Hero.jsx. However, there are significant changes in src/utils/firebase.js that appear unrelated to the stated purpose of UI/UX improvement. These changes introduce potential regressions in Firebase authentication error handling and account linking functionality, and their inclusion in this PR makes the review process more complex and increases the risk of unintended side effects.
| // Import the necessary Firebase packages | ||
| import { initializeApp } from 'firebase/app'; | ||
| import { | ||
| import { initializeApp } from "firebase/app"; | ||
| import { | ||
| getAuth, | ||
| GoogleAuthProvider, | ||
| GoogleAuthProvider, | ||
| GithubAuthProvider, | ||
| TwitterAuthProvider, | ||
| signInWithPopup, | ||
| signInWithPopup, | ||
| signOut, | ||
| onAuthStateChanged, | ||
| fetchSignInMethodsForEmail, | ||
| linkWithCredential, | ||
| EmailAuthProvider | ||
| } from 'firebase/auth'; | ||
|
|
||
| // Your Firebase configuration | ||
| // Replace these values with your actual Firebase project config | ||
| // You'll need to create a Firebase project and get these values from the Firebase console | ||
| } from "firebase/auth"; | ||
|
|
||
| // Firebase configuration (from environment variables) | ||
| const firebaseConfig = { | ||
| apiKey: import.meta.env.VITE_FIREBASE_API_KEY, | ||
| authDomain: import.meta.env.VITE_FIREBASE_AUTH_DOMAIN, | ||
| projectId: import.meta.env.VITE_FIREBASE_PROJECT_ID, | ||
| storageBucket: import.meta.env.VITE_FIREBASE_STORAGE_BUCKET, | ||
| messagingSenderId: import.meta.env.VITE_FIREBASE_MESSAGING_SENDER_ID, | ||
| appId: import.meta.env.VITE_FIREBASE_APP_ID | ||
| apiKey: import.meta.env.VITE_FIREBASE_API_KEY || "", | ||
| authDomain: import.meta.env.VITE_FIREBASE_AUTH_DOMAIN || "", | ||
| projectId: import.meta.env.VITE_FIREBASE_PROJECT_ID || "", | ||
| storageBucket: import.meta.env.VITE_FIREBASE_STORAGE_BUCKET || "", | ||
| messagingSenderId: import.meta.env.VITE_FIREBASE_MESSAGING_SENDER_ID || "", | ||
| appId: import.meta.env.VITE_FIREBASE_APP_ID || "", | ||
| }; | ||
|
|
||
| // Initialize Firebase | ||
| const app = initializeApp(firebaseConfig); | ||
| const auth = getAuth(app); | ||
| // Initialize Firebase safely | ||
| let app = null; | ||
| let auth = null; | ||
|
|
||
| try { | ||
| if (firebaseConfig.apiKey) { | ||
| app = initializeApp(firebaseConfig); | ||
| auth = getAuth(app); | ||
| } else { | ||
| console.warn("Firebase environment variables missing. Running without Firebase."); | ||
| } | ||
| } catch (error) { | ||
| console.error("Firebase initialization failed:", error); | ||
| } | ||
|
|
||
| // Providers | ||
| const googleProvider = new GoogleAuthProvider(); | ||
| const githubProvider = new GithubAuthProvider(); | ||
| const twitterProvider = new TwitterAuthProvider(); | ||
|
|
||
| // Configure Google provider (optional settings) | ||
| googleProvider.setCustomParameters({ | ||
| prompt: 'select_account' | ||
| prompt: "select_account", | ||
| }); | ||
|
|
||
| // Configure GitHub provider (optional settings) | ||
| githubProvider.setCustomParameters({ | ||
| allow_signup: 'true' | ||
| allow_signup: "true", | ||
| }); | ||
|
|
||
| // Configure Twitter provider (optional settings) | ||
| twitterProvider.setCustomParameters({ | ||
| 'lang': 'en' | ||
| lang: "en", | ||
| }); | ||
|
|
||
| // Add scopes to GitHub provider | ||
| githubProvider.addScope('user:email'); | ||
| githubProvider.addScope('read:user'); | ||
| githubProvider.addScope("user:email"); | ||
| githubProvider.addScope("read:user"); | ||
|
|
||
| // Google sign-in function | ||
| // Google sign in | ||
| export const signInWithGoogle = async () => { | ||
| try { | ||
| const result = await signInWithPopup(auth, googleProvider); | ||
| const user = result.user; | ||
|
|
||
| // Get the Firebase ID token | ||
| const idToken = await user.getIdToken(); | ||
|
|
||
| // The user info from Google | ||
| return { | ||
| user, | ||
| idToken, | ||
| // The Google OAuth access token (can be used to access Google APIs) | ||
| credential: GoogleAuthProvider.credentialFromResult(result) | ||
| }; | ||
| } catch (error) { | ||
| // Handle Errors here. | ||
| console.error("Google sign-in error:", error); | ||
|
|
||
| // If the error is about accounts with different credentials, | ||
| // store the pending credential for later linking | ||
| if (error.code === 'auth/account-exists-with-different-credential') { | ||
| const pendingCred = GoogleAuthProvider.credentialFromError(error); | ||
|
|
||
| // Store for later use if needed | ||
| if (pendingCred) { | ||
| sessionStorage.setItem('pendingCredential', JSON.stringify({ | ||
| providerId: pendingCred.providerId, | ||
| signInMethod: pendingCred.signInMethod, | ||
| email: error.customData?.email | ||
| })); | ||
| } | ||
| } | ||
|
|
||
| throw error; | ||
| } | ||
| if (!auth) throw new Error("Firebase not configured"); | ||
|
|
||
| const result = await signInWithPopup(auth, googleProvider); | ||
| const user = result.user; | ||
| const idToken = await user.getIdToken(); | ||
|
|
||
| return { | ||
| user, | ||
| idToken, | ||
| credential: GoogleAuthProvider.credentialFromResult(result), | ||
| }; | ||
| }; | ||
|
|
||
| // GitHub sign-in function | ||
| // Github sign in | ||
| export const signInWithGithub = async () => { | ||
| try { | ||
| // Configure GitHub provider to get email | ||
| githubProvider.addScope('user:email'); | ||
|
|
||
| const result = await signInWithPopup(auth, githubProvider); | ||
| const user = result.user; | ||
|
|
||
| // Get the Firebase ID token | ||
| const idToken = await user.getIdToken(); | ||
|
|
||
| // The user info from GitHub | ||
| return { | ||
| user, | ||
| idToken, | ||
| // The GitHub OAuth access token (can be used to access GitHub APIs) | ||
| credential: GithubAuthProvider.credentialFromResult(result) | ||
| }; | ||
| } catch (error) { | ||
| // Handle Errors here. | ||
| console.error("GitHub sign-in error:", error); | ||
|
|
||
| // If the error is about accounts with different credentials, | ||
| // store the pending credential for later linking | ||
| if (error.code === 'auth/account-exists-with-different-credential') { | ||
| const pendingCred = GithubAuthProvider.credentialFromError(error); | ||
|
|
||
| // Store for later use if needed | ||
| if (pendingCred) { | ||
| sessionStorage.setItem('pendingCredential', JSON.stringify({ | ||
| providerId: pendingCred.providerId, | ||
| signInMethod: pendingCred.signInMethod, | ||
| email: error.customData?.email | ||
| })); | ||
| } | ||
| } | ||
|
|
||
| throw error; | ||
| } | ||
| if (!auth) throw new Error("Firebase not configured"); | ||
|
|
||
| const result = await signInWithPopup(auth, githubProvider); | ||
| const user = result.user; | ||
| const idToken = await user.getIdToken(); | ||
|
|
||
| return { | ||
| user, | ||
| idToken, | ||
| credential: GithubAuthProvider.credentialFromResult(result), | ||
| }; | ||
| }; | ||
|
|
||
| // Twitter sign-in function | ||
| // Twitter sign in | ||
| export const signInWithTwitter = async () => { | ||
| try { | ||
| const result = await signInWithPopup(auth, twitterProvider); | ||
| const user = result.user; | ||
|
|
||
| // Get the Firebase ID token | ||
| const idToken = await user.getIdToken(); | ||
|
|
||
| // The user info from Twitter | ||
| return { | ||
| user, | ||
| idToken, | ||
| // The Twitter OAuth access token (can be used to access Twitter APIs) | ||
| credential: TwitterAuthProvider.credentialFromResult(result) | ||
| }; | ||
| } catch (error) { | ||
| // Handle Errors here. | ||
| console.error("Twitter sign-in error:", error); | ||
|
|
||
| // If the error is about accounts with different credentials, | ||
| // store the pending credential for later linking | ||
| if (error.code === 'auth/account-exists-with-different-credential') { | ||
| const pendingCred = TwitterAuthProvider.credentialFromError(error); | ||
|
|
||
| // Store for later use if needed | ||
| if (pendingCred) { | ||
| sessionStorage.setItem('pendingCredential', JSON.stringify({ | ||
| providerId: pendingCred.providerId, | ||
| signInMethod: pendingCred.signInMethod, | ||
| email: error.customData?.email | ||
| })); | ||
| } | ||
| } | ||
|
|
||
| throw error; | ||
| } | ||
| if (!auth) throw new Error("Firebase not configured"); | ||
|
|
||
| const result = await signInWithPopup(auth, twitterProvider); | ||
| const user = result.user; | ||
| const idToken = await user.getIdToken(); | ||
|
|
||
| return { | ||
| user, | ||
| idToken, | ||
| credential: TwitterAuthProvider.credentialFromResult(result), | ||
| }; | ||
| }; | ||
|
|
||
| // Sign out function | ||
| // Sign out | ||
| export const signOutUser = async () => { | ||
| try { | ||
| await signOut(auth); | ||
| return true; | ||
| } catch (error) { | ||
| console.error("Sign out error:", error); | ||
| throw error; | ||
| } | ||
| if (!auth) return true; | ||
| await signOut(auth); | ||
| }; | ||
|
|
||
| // Auth state listener | ||
| export const onAuthStateChangedListener = (callback) => { | ||
| if (!auth) return () => {}; | ||
| return onAuthStateChanged(auth, callback); | ||
| }; | ||
|
|
||
| // Function to check if an email already exists and return sign-in methods | ||
| // Check existing account | ||
| export const checkExistingAccount = async (email) => { | ||
| try { | ||
| const methods = await fetchSignInMethodsForEmail(auth, email); | ||
| return methods; | ||
| } catch (error) { | ||
| console.error("Error checking existing account:", error); | ||
| throw error; | ||
| } | ||
| if (!auth) return []; | ||
| return await fetchSignInMethodsForEmail(auth, email); | ||
| }; | ||
|
|
||
| /** | ||
| * Gets the provider name from the sign-in method string | ||
| * @param {string} method - The sign-in method string from Firebase | ||
| * @returns {string} The provider name (Google, GitHub, Email, etc.) | ||
| */ | ||
| export const getProviderFromMethod = (method) => { | ||
| switch(method) { | ||
| case 'google.com': | ||
| return 'Google'; | ||
| case 'github.com': | ||
| return 'GitHub'; | ||
| case 'twitter.com': | ||
| return 'Twitter'; | ||
| case 'password': | ||
| return 'Email/Password'; | ||
| case 'phone': | ||
| return 'Phone'; | ||
| default: | ||
| return method; | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * Attempt to link a new provider to an existing account | ||
| * This is useful when a user has accounts with different providers but the same email | ||
| * @param {object} currentUser - The currently logged-in Firebase user | ||
| * @param {object} credential - The credential from the new provider | ||
| * @returns {Promise<object>} The linked user account | ||
| */ | ||
| // Link accounts | ||
| export const linkAccounts = async (currentUser, credential) => { | ||
| try { | ||
| const result = await linkWithCredential(currentUser, credential); | ||
| return result.user; | ||
| } catch (error) { | ||
| console.error("Error linking accounts:", error); | ||
| throw error; | ||
| } | ||
| }; | ||
|
|
||
| // For all the link functions | ||
| export const linkGithubToGoogleAccount = async (credential) => { | ||
| try { | ||
| if (!auth.currentUser) { | ||
| throw new Error('No user is currently signed in'); | ||
| } | ||
|
|
||
| const result = await linkWithCredential(auth.currentUser, credential); | ||
| return result.user; | ||
| } catch (error) { | ||
| console.error('Error linking GitHub account:', error); | ||
| throw error; | ||
| } | ||
| }; | ||
|
|
||
| export const linkGoogleToGithubAccount = async (credential) => { | ||
| try { | ||
| if (!auth.currentUser) { | ||
| throw new Error('No user is currently signed in'); | ||
| } | ||
|
|
||
| const result = await linkWithCredential(auth.currentUser, credential); | ||
| return result.user; | ||
| } catch (error) { | ||
| console.error('Error linking Google account:', error); | ||
| throw error; | ||
| } | ||
| }; | ||
|
|
||
| export const linkTwitterToGoogleAccount = async (credential) => { | ||
| try { | ||
| if (!auth.currentUser) { | ||
| throw new Error('No user is currently signed in'); | ||
| } | ||
|
|
||
| const result = await linkWithCredential(auth.currentUser, credential); | ||
| return result.user; | ||
| } catch (error) { | ||
| console.error('Error linking Twitter account:', error); | ||
| throw error; | ||
| } | ||
| }; | ||
|
|
||
| export const linkTwitterToGithubAccount = async (credential) => { | ||
| try { | ||
| if (!auth.currentUser) { | ||
| throw new Error('No user is currently signed in'); | ||
| } | ||
|
|
||
| const result = await linkWithCredential(auth.currentUser, credential); | ||
| return result.user; | ||
| } catch (error) { | ||
| console.error('Error linking Twitter account:', error); | ||
| throw error; | ||
| } | ||
| }; | ||
|
|
||
| export const linkGoogleToTwitterAccount = async (credential) => { | ||
| try { | ||
| if (!auth.currentUser) { | ||
| throw new Error('No user is currently signed in'); | ||
| } | ||
|
|
||
| const result = await linkWithCredential(auth.currentUser, credential); | ||
| return result.user; | ||
| } catch (error) { | ||
| console.error('Error linking Google account:', error); | ||
| throw error; | ||
| } | ||
| }; | ||
|
|
||
| export const linkGithubToTwitterAccount = async (credential) => { | ||
| try { | ||
| if (!auth.currentUser) { | ||
| throw new Error('No user is currently signed in'); | ||
| } | ||
|
|
||
| const result = await linkWithCredential(auth.currentUser, credential); | ||
| return result.user; | ||
| } catch (error) { | ||
| console.error('Error linking GitHub account:', error); | ||
| throw error; | ||
| } | ||
| return await linkWithCredential(currentUser, credential); | ||
| }; | ||
|
|
||
| export { auth, googleProvider, githubProvider, twitterProvider }; | ||
| export { auth, googleProvider, githubProvider, twitterProvider }; No newline at end of file |
There was a problem hiding this comment.
The changes in src/utils/firebase.js are extensive and appear to be a significant refactoring of the Firebase authentication logic. These changes are unrelated to the stated purpose of this pull request, which is to improve the UI and visual polish of the Contributors button and landing page. Including unrelated functional changes in a UI/UX PR makes the review more difficult and increases the risk of introducing bugs in critical areas.
Specifically, the removal of the try-catch blocks that handled auth/account-exists-with-different-credential errors and the associated sessionStorage logic (lines 75-88, 113-128, 153-168 in the original file) represents a potential regression in how the application handles users attempting to sign in with different providers using the same email. If account linking or specific error messages for this scenario were previously supported, this functionality might now be broken or less user-friendly. The removal of EmailAuthProvider and several specific link...To...Account functions also suggests a reduction in supported authentication or linking features without clear justification in the PR description.
| /* layout polish */ | ||
|
|
||
| .main-section { | ||
| max-width: 1200px; | ||
| margin-inline: auto; | ||
| padding-inline: 1.5rem; | ||
| } | ||
|
|
||
| .hero-section { | ||
| background: radial-gradient( | ||
| circle at top, | ||
| rgba(6,182,212,0.08), | ||
| transparent 60% | ||
| ); | ||
| } | ||
|
|
||
| .contributor-card { | ||
| transition: transform 0.25s ease, box-shadow 0.25s ease; | ||
| } | ||
|
|
||
| .contributor-card:hover { | ||
| transform: translateY(-4px); | ||
| box-shadow: 0 12px 30px rgba(0,0,0,0.08); | ||
| } No newline at end of file |
There was a problem hiding this comment.
The new CSS classes .main-section, .hero-section, and .contributor-card are defined outside the @layer components block. While this works, it's generally recommended to place custom component styles within @layer components for better organization and to leverage Tailwind's processing capabilities more effectively. Additionally, the box-shadow on :hover for .contributor-card uses a fixed rgba(0,0,0,0.08) color. For better dark mode compatibility, consider using a Tailwind shadow utility class or a CSS variable that adapts to the theme.
4f7c189 to
610d325
Compare
1 participant
Pull Request Description
Summary
This pull request improves the visual styling of the Contributors button in the landing page navbar.
A new reusable
.btn-contributorsCSS class was added and applied to the Contributors link inHero.jsxto make it more modern, visually prominent, and interactive.Type of Change
Related Issues
Motivation and Context
Previously, the Contributors link appeared as plain text in the navigation bar. Since the Contributors page is important for open-source participation, making it visually distinct improves discoverability and enhances the overall landing page design.
Changes Made
.btn-contributorsstyle insrc/index.csssrc/pages/hero/Hero.jsxScreenshots (if applicable)
Before:
Plain text "Contributors" link in navbar.
After:
Styled Contributors button with improved visibility and hover interaction.
Testing
Code Quality
Deployment
Breaking Changes
This change does not introduce any breaking changes.
Additional Notes
The styling changes were implemented using Tailwind-compatible CSS and do not modify any existing functionality.
Screenshots
Before
*After
