Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix UI issues#4726 #4771

Merged
merged 7 commits into from
Oct 1, 2024
Merged

Fix UI issues#4726 #4771

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ffaa43d
made requested ui changes
lovelgeorge99 Sep 24, 2024
d0eaa92
Merge branch 'develop' of https://github.com/Giveth/giveth-dapps-v2 i…
lovelgeorge99 Sep 24, 2024
8d3a59e
remove empty link and change justifyConent
lovelgeorge99 Sep 24, 2024
ea79fe0
fixed build errors
lovelgeorge99 Sep 24, 2024
8054945
Merge branch 'develop' into fix-ui-issues#4726
lovelgeorge99 Oct 1, 2024
a5c7688
Merge branch 'develop' into fix-ui-issues#4726
lovelgeorge99 Oct 1, 2024
bfa59fa
Merge branch 'develop' into fix-ui-issues#4726
lovelgeorge99 Oct 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/badges/ShareLikeBadge.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const Wrapper = styled(FlexCenter)`
const BadgeText = styled(ButtonText)<{ $fromDonate?: boolean | null }>`
color: ${props =>
props.$fromDonate ? brandColors.pinky[500] : neutralColors.gray[700]};
text-transform: ${props => (props.$fromDonate ? 'none' : 'uppercase')};
text-transform: ${props => (props.$fromDonate ? 'none' : '')};
margin: 0 auto;
`;

Expand Down
4 changes: 1 addition & 3 deletions src/components/views/project/ProjectBadges.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,9 @@ const ProjectBadges = () => {
};

const CustomFlex = styled(Flex)`
overflow-x: scroll;
overflow-y: hidden;
white-space: nowrap;
margin-bottom: -3px;
padding-top: 8px;
margin-bottom: 24px;
`;

export const TooltipContent = styled.div`
Expand Down
4 changes: 2 additions & 2 deletions src/components/views/project/ProjectHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ const ProjectImage = styled.img`
border-radius: 16px;
width: 100%;
object-fit: cover; // Ensures the image covers the entire container
height: 380px;
height: 430px;
position: relative;
`;

Expand All @@ -65,7 +65,7 @@ const GradientOverlay = styled.div`
top: 0;
left: 0;
width: 100%;
height: 380px;
height: 430px;
background: linear-gradient(
to top,
rgba(1, 1, 27, 0.6),
Expand Down
4 changes: 2 additions & 2 deletions src/components/views/project/ProjectIndex.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ const ProjectIndex: FC<IProjectBySlug> = () => {
orgLabel={projectData?.organization?.label}
/>
<Row>
<Col xs={12} md={8} lg={8.5}>
<Col xs={12} md={8} lg={8.5} style={{ margin: '0' }}>
<ProjectHeader />
{isMobile && isAdmin && (
<MobileActionsContainer
Expand All @@ -202,7 +202,7 @@ const ProjectIndex: FC<IProjectBySlug> = () => {
)}
<ProjectGIVbackToast />
</Col>
<Col xs={12} md={4} lg={3.5}>
<Col xs={12} md={4} lg={3.5} style={{ margin: '0' }}>
<ProjectActionCard />
</Col>
{isDraft && (
Expand Down
17 changes: 1 addition & 16 deletions src/components/views/project/projectActionCard/ProjectActionCard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,7 @@ import {
mediaQueries,
neutralColors,
Flex,
Subline,
brandColors,
IconChevronRight16,
} from '@giveth/ui-design-system';
import Link from 'next/link';
import styled from 'styled-components';
Expand Down Expand Up @@ -57,20 +55,7 @@ const ProjectActionInnerCard = () => {
)}
{!isMobile && !isAdmin && <ProjectPublicActions />}
{isAdmin && <ProjectStats />}
<Link href={Routes.Onboarding + '/donors'}>
<LearnLink
$alignItems='center'
$justifyContent='center'
gap='2px'
>
<Subline>
{formatMessage({
id: 'label.learn_more_about_donating_on_giveth',
})}
</Subline>
<IconChevronRight16 />
</LearnLink>
</Link>
<Link href={Routes.Onboarding + '/donors'}></Link>
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove the empty Link component

The Link component on line 58 is empty and serves no purpose. This appears to be a remnant from the removal of the "Learn More" link functionality.

Consider removing this line entirely:

-<Link href={Routes.Onboarding + '/donors'}></Link>

If there's a specific reason to keep a link to the donors onboarding page, consider adding it elsewhere in the component with proper content and accessibility considerations.

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.

Suggested change
<Link href={Routes.Onboarding + '/donors'}></Link>

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lovelgeorge99 can you check why this link is empty? thx

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes i removed the empty link on later commit.

</>
);
};
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/project/projectActionCard/ProjectPublicActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export const ProjectPublicActions = () => {
{reaction?.userId && reaction?.userId === user?.id ? (
<IconBookmarkFilled16 color={brandColors.pinky[500]} />
) : (
<IconBookmark16 />
<IconBookmark16 color='#525f7f' />
)}
</StyledBadgeButton>
</BadgeWrapper>
Expand Down
3 changes: 1 addition & 2 deletions src/components/views/project/projectActionCard/QFSection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ const QFSection: FC<IQFSectionProps> = ({ projectData }) => {
);

return (
<DonationSectionWrapper gap={isOnDonatePage ? '8px' : '24px'}>
<DonationSectionWrapper gap={'8px'}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Inconsistent gap values in DonationSectionWrapper

Multiple instances of DonationSectionWrapper use different gap values (24px, 16px, 8px) across the codebase:

  • src/components/views/project/projectActionCard/DonationSection.tsx uses gap='24px'
  • src/components/views/donate/DonatePageProjectDescription.tsx uses gap='16px'
  • src/components/views/project/projectActionCard/QFSection.tsx uses gap={'8px'}

This inconsistency may lead to uneven spacing in the UI. Additionally, the removal of margin-bottom for the Description component was not detected in the codebase.

Analysis chain

Simplified gap property in DonationSectionWrapper

The gap property in the DonationSectionWrapper component has been simplified from a conditional expression to a fixed value of '8px'. This change ensures consistent spacing across all use cases of the component.

While this change improves consistency, it's important to verify that it doesn't negatively impact the layout in scenarios where a different gap was previously applied. Please run the following script to check for any other occurrences of DonationSectionWrapper with different gap values:

Additionally, the AI summary mentions the removal of a margin-bottom for the Description component, which is not visible in the provided code. To verify this:

Please review the results of these searches to ensure that the changes don't introduce any unintended layout issues.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other occurrences of DonationSectionWrapper with gap prop
rg --type typescript --type tsx 'DonationSectionWrapper.*gap' src

Length of output: 102

Script:

#!/bin/bash
# Search for Description styled component with margin-bottom
rg --type typescript --type tsx 'const Description.*margin-bottom' src

Length of output: 107

Script:

#!/bin/bash
# Search for other occurrences of DonationSectionWrapper with gap prop
rg --type ts --type tsx 'DonationSectionWrapper.*gap' src

Length of output: 87

Script:

#!/bin/bash
# Search for Description styled component with margin-bottom
rg --type ts --type tsx 'const Description.*margin-bottom' src

Length of output: 92

Script:

#!/bin/bash
# Search for other occurrences of DonationSectionWrapper with gap prop in .ts and .tsx files
rg 'DonationSectionWrapper.*gap' --glob '*.ts' --glob '*.tsx' src

Length of output: 371

Script:

#!/bin/bash
# Search for Description styled component with margin-bottom in .ts and .tsx files
rg 'const Description.*margin-bottom' --glob '*.ts' --glob '*.tsx' src

Length of output: 72

{isOnDonatePage && (
<>
<Link href={projectLink}>
Expand Down Expand Up @@ -302,7 +302,6 @@ const Amount = styled(H3)`

const Description = styled(Caption)`
color: ${neutralColors.gray[700]};
margin-bottom: 24px;
white-space: nowrap;
& > div {
color: ${neutralColors.gray[900]};
Expand Down
Loading