add sticky banner #4846
add sticky banner #4846
Conversation
add sticky 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 changes in this pull request focus on the Changes
Suggested reviewers
Poem
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: 2
📜 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] 116-116:
Replace·with⏎↹↹
[failure] 117-117:
Replace·with⏎↹↹
[failure] 123-123:
Delete··
[failure] 125-125:
Delete··
[failure] 128-128:
Delete··
[failure] 131-131:
Replace··with↹
[failure] 133-133:
Delete··
[failure] 135-135:
Replace··with↹
[failure] 136-136:
Insert↹
[failure] 137-137:
Replace··with↹↹
🔇 Additional comments (1)
src/components/PassportBanner.tsx (1)
266-268: Verify the z-index value to prevent stacking issuesSetting
z-index: 1000might cause stacking context problems with other components. Ensure that this value does not conflict with other elements in the application.To confirm that the
z-indexvalue does not overlap with other components, run the following script to search for highz-indexvalues in the codebase:#!/bin/bash # Description: Find all instances of z-index with values 1000 or higher. # Search for z-index values 1000 or higher rg 'z-index\s*:\s*\d{4,}' -I --glob '!node_modules/*'
src/components/PassportBanner.tsx
Outdated
| return null; // Or return a spinner or loading message if you'd like | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Provide a loading indicator instead of returning null
Currently, when data is loading, the component returns null, resulting in no UI feedback. Consider displaying a loading spinner or placeholder content to enhance user experience during data fetch.
🧰 Tools
🪛 GitHub Check: build
[failure] 131-131:
Replace··with↹
src/components/PassportBanner.tsx
Outdated
| ? smallFormatDate( | ||
| new Date( | ||
| currentRound?.beginDate, | ||
| ), | ||
| ) | ||
| : undefined, | ||
| }, | ||
| )} | ||
| {currentRound && | ||
| qfEligibilityState === | ||
| EQFElegibilityState.RECHECK_ELIGIBILITY && ( | ||
| <> | ||
| {' '} | ||
| <strong> | ||
| {new Date(currentRound.endDate) | ||
| .toLocaleString(locale || 'en-US', { | ||
| day: 'numeric', | ||
| month: 'short', | ||
| }) | ||
| .replace(/,/g, '')} | ||
| </strong> | ||
| </> | ||
| )} | ||
| </P> | ||
| </Flex> | ||
| {qfEligibilityState === | ||
| EQFElegibilityState.CHECK_ELIGIBILITY && ( | ||
| <StyledLink onClick={() => fetchUserMBDScore()}> | ||
| <GLink> | ||
| {formatMessage({ | ||
| id: 'qf_donor_eligibility.banner.link.check_eligibility', | ||
| })} | ||
| </GLink> | ||
| </StyledLink> | ||
| )} | ||
| {qfEligibilityState === | ||
| EQFElegibilityState.RECHECK_ELIGIBILITY && ( | ||
| <StyledLink onClick={() => setShowModal(true)}> | ||
| <GLink> | ||
| {formatMessage({ | ||
| id: 'qf_donor_eligibility.banner.link.recheck_eligibility', | ||
| })} | ||
| </GLink> | ||
| </StyledLink> | ||
| )} | ||
| {qfEligibilityState === EQFElegibilityState.PROCESSING && ( | ||
| <StyledStatus> | ||
| {formatMessage({ | ||
| id: 'label.processing', | ||
| })} | ||
| <Spinner color={brandColors.mustard[600]} size={25} /> | ||
| </StyledStatus> | ||
| )} | ||
| {qfEligibilityState === | ||
| EQFElegibilityState.MORE_INFO_NEEDED && ( | ||
| <StyledLink onClick={() => setShowModal(true)}> | ||
| <GLink> | ||
| {formatMessage({ | ||
| id: 'label.add_more_info', | ||
| })} | ||
| </GLink> | ||
| </StyledLink> | ||
| )} | ||
| {qfEligibilityState === EQFElegibilityState.NOT_SIGNED && ( | ||
| <StyledLink onClick={() => setSignWithWallet(true)}> | ||
| <GLink> | ||
| {formatMessage({ | ||
| id: 'label.sign_message', | ||
| })} | ||
| </GLink> | ||
| <IconWalletOutline16 /> | ||
| </StyledLink> | ||
| )} | ||
| </PassportBannerWrapper> | ||
| {showModal && ( | ||
| <PassportModal | ||
| qfEligibilityState={qfEligibilityState} | ||
| passportState={passportState} | ||
| passportScore={passportScore} | ||
| currentRound={currentRound} | ||
| setShowModal={setShowModal} | ||
| updateState={updateState} | ||
| refreshScore={refreshScore} | ||
| handleSign={handleSign} | ||
| /> | ||
| )} | ||
| {signWithWallet && ( | ||
| <SignWithWalletModal | ||
| isGSafeConnector={isGSafeConnector} | ||
| setShowModal={() => { | ||
| setSignWithWallet(false); | ||
| }} | ||
| /> | ||
| )} | ||
| </> | ||
| ) : ( | ||
| <> | ||
| <PassportBannerWrapper | ||
| $bgColor={PassportBannerData[qfEligibilityState].bg} | ||
| $bgColor={PassportBannerData[qfEligibilityState].bg} |
There was a problem hiding this comment.
Fix indentation and remove extra whitespace
Lines 116-137 contain inconsistent indentation and unnecessary blank lines. Please adjust the code to align with the project's formatting guidelines for better readability.
Apply this diff to correct the formatting:
- const { info, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport();
- const { currentRound, passportState, passportScore, qfEligibilityState } = info;
-
-
- const isGSafeConnector = connector?.id === 'safe';
-
-
- // 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
- }
+const { info, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport();
+const { currentRound, passportState, passportScore, qfEligibilityState } = info;
+
+const isGSafeConnector = connector?.id === 'safe';
+
+// 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
+}📝 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.
| const { info, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport(); | |
| const { currentRound, passportState, passportScore, qfEligibilityState } = info; | |
| const { formatMessage, locale } = useIntl(); | |
| const { connector } = useAccount(); | |
| const { isOnSolana, handleSingOutAndSignInWithEVM } = useGeneralWallet(); | |
| const [showModal, setShowModal] = useState<boolean>(false); | |
| const [signWithWallet, setSignWithWallet] = useState<boolean>(false); | |
| const isGSafeConnector = connector?.id === 'safe'; | |
| // 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 | |
| } | |
| return !isOnSolana ? ( | |
| <> | |
| <PassportBannerWrapper | |
| $bgColor={PassportBannerData[qfEligibilityState].bg} | |
| > | |
| <Flex gap='8px' $alignItems='center'> | |
| <IconWrapper> | |
| {PassportBannerData[qfEligibilityState].icon} | |
| </IconWrapper> | |
| <P> | |
| {formatMessage( | |
| { | |
| id: PassportBannerData[qfEligibilityState] | |
| .content, | |
| }, | |
| { | |
| data: | |
| qfEligibilityState === | |
| EQFElegibilityState.NOT_STARTED && | |
| currentRound | |
| ? smallFormatDate( | |
| new Date( | |
| currentRound?.beginDate, | |
| ), | |
| ) | |
| : undefined, | |
| }, | |
| )} | |
| {currentRound && | |
| qfEligibilityState === | |
| EQFElegibilityState.RECHECK_ELIGIBILITY && ( | |
| <> | |
| {' '} | |
| <strong> | |
| {new Date(currentRound.endDate) | |
| .toLocaleString(locale || 'en-US', { | |
| day: 'numeric', | |
| month: 'short', | |
| }) | |
| .replace(/,/g, '')} | |
| </strong> | |
| </> | |
| )} | |
| </P> | |
| </Flex> | |
| {qfEligibilityState === | |
| EQFElegibilityState.CHECK_ELIGIBILITY && ( | |
| <StyledLink onClick={() => fetchUserMBDScore()}> | |
| <GLink> | |
| {formatMessage({ | |
| id: 'qf_donor_eligibility.banner.link.check_eligibility', | |
| })} | |
| </GLink> | |
| </StyledLink> | |
| )} | |
| {qfEligibilityState === | |
| EQFElegibilityState.RECHECK_ELIGIBILITY && ( | |
| <StyledLink onClick={() => setShowModal(true)}> | |
| <GLink> | |
| {formatMessage({ | |
| id: 'qf_donor_eligibility.banner.link.recheck_eligibility', | |
| })} | |
| </GLink> | |
| </StyledLink> | |
| )} | |
| {qfEligibilityState === EQFElegibilityState.PROCESSING && ( | |
| <StyledStatus> | |
| {formatMessage({ | |
| id: 'label.processing', | |
| })} | |
| <Spinner color={brandColors.mustard[600]} size={25} /> | |
| </StyledStatus> | |
| )} | |
| {qfEligibilityState === | |
| EQFElegibilityState.MORE_INFO_NEEDED && ( | |
| <StyledLink onClick={() => setShowModal(true)}> | |
| <GLink> | |
| {formatMessage({ | |
| id: 'label.add_more_info', | |
| })} | |
| </GLink> | |
| </StyledLink> | |
| )} | |
| {qfEligibilityState === EQFElegibilityState.NOT_SIGNED && ( | |
| <StyledLink onClick={() => setSignWithWallet(true)}> | |
| <GLink> | |
| {formatMessage({ | |
| id: 'label.sign_message', | |
| })} | |
| </GLink> | |
| <IconWalletOutline16 /> | |
| </StyledLink> | |
| )} | |
| </PassportBannerWrapper> | |
| {showModal && ( | |
| <PassportModal | |
| qfEligibilityState={qfEligibilityState} | |
| passportState={passportState} | |
| passportScore={passportScore} | |
| currentRound={currentRound} | |
| setShowModal={setShowModal} | |
| updateState={updateState} | |
| refreshScore={refreshScore} | |
| handleSign={handleSign} | |
| /> | |
| )} | |
| {signWithWallet && ( | |
| <SignWithWalletModal | |
| isGSafeConnector={isGSafeConnector} | |
| setShowModal={() => { | |
| setSignWithWallet(false); | |
| }} | |
| /> | |
| )} | |
| </> | |
| ) : ( | |
| <> | |
| <PassportBannerWrapper | |
| $bgColor={PassportBannerData[qfEligibilityState].bg} | |
| $bgColor={PassportBannerData[qfEligibilityState].bg} | |
| const { info, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport(); | |
| const { currentRound, passportState, passportScore, qfEligibilityState } = info; | |
| const { formatMessage, locale } = useIntl(); | |
| const { connector } = useAccount(); | |
| const { isOnSolana, handleSingOutAndSignInWithEVM } = useGeneralWallet(); | |
| const [showModal, setShowModal] = useState<boolean>(false); | |
| const [signWithWallet, setSignWithWallet] = useState<boolean>(false); | |
| const isGSafeConnector = connector?.id === 'safe'; | |
| // 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 | |
| } | |
| return !isOnSolana ? ( | |
| <> | |
| <PassportBannerWrapper | |
| $bgColor={PassportBannerData[qfEligibilityState].bg} |
🧰 Tools
🪛 GitHub Check: build
[failure] 116-116:
Replace·with⏎↹↹
[failure] 117-117:
Replace·with⏎↹↹
[failure] 123-123:
Delete··
[failure] 125-125:
Delete··
[failure] 128-128:
Delete··
[failure] 131-131:
Replace··with↹
[failure] 133-133:
Delete··
[failure] 135-135:
Replace··with↹
[failure] 136-136:
Insert↹
[failure] 137-137:
Replace··with↹↹
|
@maryjaf on what task this PR is referring? thx |
|
@maryjaf you have some prettier issues in this PR, please install in VS code editor Prettier - Code formatter and try to format code after that. After install re-open your editor.
|
edit some formatting issue
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/components/PassportBanner.tsx (2)
116-117: Fix indentation for better readabilityThe indentation of these lines should be adjusted to match the rest of the code. Consider using an autoformatter to maintain consistent indentation throughout the file.
Apply this diff to correct the indentation:
- const { info, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport(); - const { currentRound, passportState, passportScore, qfEligibilityState } = info; +const { info, updateState, fetchUserMBDScore, handleSign, refreshScore } = usePassport(); +const { currentRound, passportState, passportScore, qfEligibilityState } = info;🧰 Tools
🪛 GitHub Check: build
[failure] 116-116:
Replace·with⏎↹↹
[failure] 117-117:
Replace·with⏎↹↹
171-215: Well-implemented conditional rendering with i18n supportThe conditional rendering of action links based on different states is well-implemented. The use of
formatMessagefor internationalization is commendable.Consider extracting the repeated StyledLink pattern into a separate component to reduce code duplication and improve maintainability. For example:
const ActionLink = ({ onClick, messageId, icon }) => ( <StyledLink onClick={onClick}> <GLink>{formatMessage({ id: messageId })}</GLink> {icon} </StyledLink> );Then use it like:
{qfEligibilityState === EQFElegibilityState.CHECK_ELIGIBILITY && ( <ActionLink onClick={() => fetchUserMBDScore()} messageId="qf_donor_eligibility.banner.link.check_eligibility" /> )}
📜 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] 116-116:
Replace·with⏎↹↹
[failure] 117-117:
Replace·with⏎↹↹
[failure] 123-123:
Delete··
[failure] 125-125:
Delete··
[failure] 128-128:
Delete··
[failure] 131-131:
Replace··with↹
[failure] 133-133:
Delete··
[failure] 135-135:
Replace··with↹
[failure] 136-136:
Insert↹
[failure] 137-137:
Replace··with↹↹
🔇 Additional comments (4)
src/components/PassportBanner.tsx (4)
139-170: Well-structured and maintainable JSXThe JSX structure for the banner content is well-organized, using appropriate components and dynamic content rendering. This approach enhances readability and maintainability.
217-250: Well-implemented modal rendering and platform-specific contentThe conditional rendering of modals and the Solana-specific content is well-implemented. The use of state variables to control modal visibility is appropriate, and the platform-specific content enhances the user experience for Solana users.
Line range hint
1-300: Overall well-implemented changes with room for minor improvementsThe changes to the PassportBanner component are generally well-implemented, improving loading state handling, banner visibility, and code structure. Consider the suggested minor improvements for loading indicator, action link component extraction, and z-index verification to further enhance the component's quality and maintainability.
🧰 Tools
🪛 GitHub Check: build
[failure] 116-116:
Replace·with⏎↹↹
[failure] 117-117:
Replace·with⏎↹↹
[failure] 123-123:
Delete··
[failure] 125-125:
Delete··
[failure] 128-128:
Delete··
[failure] 131-131:
Replace··with↹
[failure] 133-133:
Delete··
[failure] 135-135:
Replace··with↹
[failure] 136-136:
Insert↹
[failure] 137-137:
Replace··with↹↹
260-271: Improved banner visibility with sticky positioningThe update to use a sticky position for the PassportBannerWrapper is a good improvement for user experience, ensuring the banner remains visible during scrolling.
Verify that the z-index value of 1000 is appropriate in the context of the entire application to avoid any potential layering issues with other components. Run the following script to check for other high z-index values:
If this script returns other high z-index values, consider adjusting the value for PassportBannerWrapper to ensure proper layering.
✅ Verification successful
z-index value verification for PassportBannerWrapper
The z-index value of 1000 in
src/components/PassportBanner.tsxis consistent with other components in the codebase and should not cause layering issues.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for high z-index values in the codebase # Search for z-index declarations with values >= 1000 rg 'z-index:\s*(\d{4,}|[1-9]\d{3,})' --type css --type ts --type tsxLength of output: 98
Script:
#!/bin/bash # Description: Check for high z-index values in the codebase # Search for z-index declarations with values >= 1000 rg 'z-index:\s*(\d{4,}|[1-9]\d{3,})' --type css --type ts --type typescriptreactLength of output: 122
Script:
#!/bin/bash # Description: Check for high z-index values in the codebase # Search for z-index declarations with values >= 1000 rg 'z-index:\s*(\d{4,}|[1-9]\d{3,})' --type css --type ts --type tsxLength of output: 98
Script:
#!/bin/bash # Description: Check for high z-index values in the codebase # Search for z-index declarations with values >= 1000 in CSS, TS, and TSX files rg 'z-index:\s*(\d{4,}|[1-9]\d{3,})' --glob '*.css' --glob '*.ts' --glob '*.tsx'Length of output: 715
src/components/PassportBanner.tsx
Outdated
| // 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.
🛠️ Refactor suggestion
Consider implementing a loading indicator
While checking for loading state is good, returning null doesn't provide feedback to the user. Consider implementing a loading indicator or placeholder content to enhance the user experience during data fetching.
Here's a suggestion for improvement:
if (isLoading) {
return <LoadingIndicator />; // Replace with your preferred loading component
}🧰 Tools
🪛 GitHub Check: build
[failure] 128-128:
Delete··
[failure] 131-131:
Replace··with↹
add prettier fixes
add prettier fixes
add sticky passport banner
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements