add sticky style #4848
add sticky style #4848
Conversation
add sticky style for passport banner
|
@maryjaf is attempting to deploy a commit to the Giveth Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe pull request introduces modifications to the Changes
Possibly related PRs
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
src/components/PassportBanner.tsx (2)
130-131: Consider providing user feedback during loading stateReturning
nullwhen the data is loading might result in an empty space without any user feedback. Consider displaying a spinner or a placeholder message to inform users that content is loading.For example:
if (isLoading) { - return null; // Or return a spinner or loading message if you'd like + return <Spinner size={25} />; }🧰 Tools
🪛 GitHub Check: build
[failure] 130-130:
Replace··with↹
115-116: Duplicate variable declarations in close proximityIt appears that variables
currentRound,passportState,passportScore, andqfEligibilityStateare destructured frominforight after importinginfofromusePassport(). Consider combining these declarations for conciseness.Apply this diff:
- const { info, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport(); - const { currentRound, passportState, passportScore, qfEligibilityState } = info; + const { info: { currentRound, passportState, passportScore, qfEligibilityState }, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport();🧰 Tools
🪛 GitHub Check: build
[failure] 115-115:
Replace·with⏎↹↹
[failure] 116-116:
Replace·with⏎↹↹
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/components/PassportBanner.tsx (1 hunks)
🧰 Additional context used
🪛 GitHub Check: build
src/components/PassportBanner.tsx
[failure] 115-115:
Replace·with⏎↹↹
[failure] 116-116:
Replace·with⏎↹↹
[failure] 122-122:
Delete··
[failure] 124-124:
Delete··
[failure] 127-127:
Delete··
[failure] 130-130:
Replace··with↹
[failure] 132-132:
Delete··
[failure] 134-134:
Replace··with↹
[failure] 135-135:
Insert↹
[failure] 136-136:
Replace··with↹↹
🔇 Additional comments (2)
src/components/PassportBanner.tsx (2)
266-269: Verify cross-browser compatibility ofposition: stickyThe use of
position: stickycan have inconsistent support across different browsers, especially older ones. Ensure that the sticky positioning works as intended across all supported browsers and devices.
266-271: LGTMThe changes to
PassportBannerWrapperimplementing sticky positioning enhance the user experience by keeping the banner visible during scrolling. The use oftop: 0andz-index: 1000is appropriate to maintain visibility.
|
|
||
| const isGSafeConnector = connector?.id === 'safe'; | ||
|
|
||
|
|
There was a problem hiding this comment.
Fix formatting issues in blank lines and indentation
Static analysis tools have detected extra spaces on blank lines and incorrect indentation. For code cleanliness and adherence to the project's coding standards, please remove unnecessary whitespace and adjust indentation where necessary.
Apply this diff:
-
-
-
const isGSafeConnector = connector?.id === 'safe';
-
// Check if the eligibility state or current round is not loaded yet
const isLoading = qfEligibilityState === undefined || currentRound === undefined;
-
// Only render the banner when the data is available
if (isLoading) {
- return null; // Or return a spinner or loading message if you'd like
+ return null; // Or return a spinner or loading message if you'd like
}
-
return !isOnSolana ? (
+ <>
<PassportBannerWrapper
- $bgColor={PassportBannerData[qfEligibilityState].bg}
+ $bgColor={PassportBannerData[qfEligibilityState].bg}
>Also applies to: 127-127, 130-130, 132-132, 134-134, 136-136
🧰 Tools
🪛 GitHub Check: build
[failure] 122-122:
Delete··
[failure] 124-124:
Delete··
| // Check if the eligibility state or current round is not loaded yet | ||
| const isLoading = !qfEligibilityState || !currentRound; | ||
|
|
||
| // Only render the banner when the data is available | ||
| if (isLoading) { | ||
| return null; // Or return a spinner or loading message if you'd like | ||
| } |
There was a problem hiding this comment.
Ensure correct checking of undefined values to prevent logical errors
Using !qfEligibilityState may lead to unintended behavior if qfEligibilityState can have valid falsy values (e.g., 0). To accurately check if qfEligibilityState or currentRound is undefined, consider using explicit comparisons.
Apply this diff to adjust the condition:
- const isLoading = !qfEligibilityState || !currentRound;
+ const isLoading = qfEligibilityState === undefined || currentRound === undefined;📝 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.
| // Check if the eligibility state or current round is not loaded yet | |
| const isLoading = !qfEligibilityState || !currentRound; | |
| // Only render the banner when the data is available | |
| if (isLoading) { | |
| return null; // Or return a spinner or loading message if you'd like | |
| } | |
| // Check if the eligibility state or current round is not loaded yet | |
| const isLoading = qfEligibilityState === undefined || currentRound === undefined; | |
| // Only render the banner when the data is available | |
| if (isLoading) { | |
| return null; // Or return a spinner or loading message if you'd like | |
| } |
🧰 Tools
🪛 GitHub Check: build
[failure] 127-127:
Delete··
[failure] 130-130:
Replace··with↹
| const { info, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport(); | ||
| const { currentRound, passportState, passportScore, qfEligibilityState } = info; |
There was a problem hiding this comment.
Fix formatting issues as indicated by static analysis
Static analysis tools have identified formatting inconsistencies on these lines. Please adjust the indentation and spacing to align with the project's code style guidelines.
Apply this diff to correct the formatting:
+
const { info, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport();
const { currentRound, passportState, passportScore, qfEligibilityState } = info;Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 GitHub Check: build
[failure] 115-115:
Replace·with⏎↹↹
[failure] 116-116:
Replace·with⏎↹↹
| {qfEligibilityState === EQFElegibilityState.CHECK_ELIGIBILITY && ( | ||
| <StyledLink onClick={() => fetchUserMBDScore()}> | ||
| <GLink> | ||
| {formatMessage({ | ||
| id: 'label.to_activate_your_gitcoin_passport', | ||
| id: 'qf_donor_eligibility.banner.link.check_eligibility', | ||
| })} | ||
| </P> | ||
| <StyledP onClick={handleSingOutAndSignInWithEVM}> | ||
| </GLink> | ||
| </StyledLink> | ||
| )} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid duplicating conditional rendering for similar eligibility states
The rendering logic for CHECK_ELIGIBILITY and RECHECK_ELIGIBILITY states is very similar. Consider refactoring to reduce code duplication.
Apply this diff:
{ (qfEligibilityState === EQFElegibilityState.CHECK_ELIGIBILITY || qfEligibilityState === EQFElegibilityState.RECHECK_ELIGIBILITY) && (
<StyledLink onClick={() => {
+ qfEligibilityState === EQFElegibilityState.CHECK_ELIGIBILITY ? fetchUserMBDScore() : setShowModal(true);
+ }}>
<GLink>
{formatMessage({
- id: 'qf_donor_eligibility.banner.link.check_eligibility',
+ id: qfEligibilityState === EQFElegibilityState.CHECK_ELIGIBILITY
+ ? 'qf_donor_eligibility.banner.link.check_eligibility'
+ : 'qf_donor_eligibility.banner.link.recheck_eligibility',
})}
</GLink>
</StyledLink>
)}📝 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.
| {qfEligibilityState === EQFElegibilityState.CHECK_ELIGIBILITY && ( | |
| <StyledLink onClick={() => fetchUserMBDScore()}> | |
| <GLink> | |
| {formatMessage({ | |
| id: 'label.to_activate_your_gitcoin_passport', | |
| id: 'qf_donor_eligibility.banner.link.check_eligibility', | |
| })} | |
| </P> | |
| <StyledP onClick={handleSingOutAndSignInWithEVM}> | |
| </GLink> | |
| </StyledLink> | |
| )} | |
| {(qfEligibilityState === EQFElegibilityState.CHECK_ELIGIBILITY || qfEligibilityState === EQFElegibilityState.RECHECK_ELIGIBILITY) && ( | |
| <StyledLink onClick={() => { | |
| qfEligibilityState === EQFElegibilityState.CHECK_ELIGIBILITY ? fetchUserMBDScore() : setShowModal(true); | |
| }}> | |
| <GLink> | |
| {formatMessage({ | |
| id: qfEligibilityState === EQFElegibilityState.CHECK_ELIGIBILITY | |
| ? 'qf_donor_eligibility.banner.link.check_eligibility' | |
| : 'qf_donor_eligibility.banner.link.recheck_eligibility', | |
| })} | |
| </GLink> | |
| </StyledLink> | |
| )} |
add sticky style for passport banner
#4520
Summary by CodeRabbit
New Features
Bug Fixes