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.

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

Fix: Align Project Details and Statistics blocks #607

Merged
merged 10 commits into from
Jan 30, 2025

Conversation

YourBroCode
Copy link
Contributor

@YourBroCode YourBroCode commented Jan 25, 2025

Resolves #549

fixed the vertical padding and aligned the contents into two columns of equal width.
Here is the screenshot of fixed blocks:
Screenshot 2025-01-25 at 2 21 16 AM

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.

A couple of suggestions:

  • you don't need to change the statistics block size
  • the "Organization" text can be removed to make it look more organic

@YourBroCode
Copy link
Contributor Author

YourBroCode commented Jan 26, 2025

@arkid15r I have made the required changes. please review:

Screenshot 2025-01-27 at 3 06 13 AM

update:

I tried to test my code locally via make test . some test cases are failing. I found that i can not remove the "Organization" text because the internal test case is checking "OWASP" text.

please suggest what should i do ?

@YourBroCode YourBroCode requested a review from arkid15r January 27, 2025 05:50
@arkid15r arkid15r enabled auto-merge January 27, 2025 20:26
@arkid15r
Copy link
Collaborator

@arkid15r I have made the required changes. please review:
Screenshot 2025-01-27 at 3 06 13 AM

update:

I tried to test my code locally via make test . some test cases are failing. I found that i can not remove the "Organization" text because the internal test case is checking "OWASP" text.

please suggest what should i do ?

You need to run pre-commit and update the tests based on the new behavior/look.

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.

Please fix the style and tests

auto-merge was automatically disabled January 28, 2025 15:07

Head branch was pushed to by a user without write access

@YourBroCode YourBroCode requested a review from arkid15r January 28, 2025 15:11
@YourBroCode YourBroCode marked this pull request as draft January 28, 2025 22:07
@YourBroCode YourBroCode marked this pull request as ready for review January 29, 2025 06:03
@arkid15r arkid15r enabled auto-merge January 30, 2025 01:11
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 Jan 30, 2025
Merged via the queue into OWASP:main with commit 9bbc43e Jan 30, 2025
13 checks passed
@YourBroCode YourBroCode deleted the fix/align-blocks branch January 30, 2025 06:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve project details and statistics blocks
2 participants