Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

Update card layout #780

Merged
merged 8 commits into from
Feb 8, 2025
Merged

Update card layout #780

merged 8 commits into from
Feb 8, 2025

Conversation

yashgoyal0110
Copy link
Contributor

fixes: #651

Screen.Recording.2025-02-09.at.12.57.43.AM.mp4

arkid15r
arkid15r requested changes Feb 8, 2025
View reviewed changes
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

Looks good, let's change the order: stars, forks, contributors.

@yashgoyal0110
Copy link
Contributor Author

Looks good, let's change the order: stars, forks, contributors.

Screenshot 2025-02-09 at 1 18 51 AM

done

Dishant1804
Dishant1804 approved these changes Feb 8, 2025
View reviewed changes
Copy link
Collaborator

@Dishant1804 Dishant1804 left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

arkid15r
arkid15r requested changes Feb 8, 2025
View reviewed changes
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

Could you also align it vertically with other content?

Screenshot 2025-02-08 at 12 41 06 PM

frontend/src/utils/data.ts Outdated Show resolved Hide resolved
frontend/src/utils/data.ts Outdated Show resolved Hide resolved
frontend/src/utils/data.ts Outdated Show resolved Hide resolved
@arkid15r arkid15r changed the title card-layout Update card layout Feb 8, 2025
arkid15r
arkid15r reviewed Feb 8, 2025
View reviewed changes
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

LGTM, please fix the empty icons case before the merge.

frontend/src/components/Card.tsx Outdated Show resolved Hide resolved
arkid15r
arkid15r approved these changes Feb 8, 2025
View reviewed changes
Copy link
Collaborator

@arkid15r arkid15r left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@arkid15r arkid15r added this pull request to the merge queue Feb 8, 2025
Merged via the queue into OWASP:main with commit fca531f Feb 8, 2025
13 checks passed
@arkid15r arkid15r mentioned this pull request Feb 8, 2025
Open
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@arkid15r arkid15r arkid15r approved these changes

@Dishant1804 Dishant1804 Dishant1804 approved these changes

@kasya kasya Awaiting requested review from kasya kasya is a code owner

@aramattamara aramattamara Awaiting requested review from aramattamara

Assignees
No one assigned
Labels
frontend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update Card component
3 participants
@yashgoyal0110 @arkid15r @Dishant1804