Skip to content

fix/Loader to follow OWASP branding Guidelines - #4508

Merged
arkid15r merged 3 commits into
OWASP:mainfrom
Saurabh-2607:fix/loader
Apr 11, 2026
Merged

arkid15r merged 3 commits into
OWASP:mainfrom
Saurabh-2607:fix/loader

Conversation

@Saurabh-2607

Copy link
Copy Markdown
Contributor

Proposed change

Resolves #4338

This PR fixes the issue where the OWASP logo rotates together with the loading spinner.

According to OWASP branding guidelines, the logo should not be altered in appearance or orientation. The previous implementation applied the rotation animation to the entire spinner container, causing the logo to rotate along with it.

Changes made

  • Separated the spinner ring and the OWASP logo into different elements.
  • Applied the rotation animation only to the spinner ring.
  • Kept the OWASP logo static and centered to maintain branding compliance.
  • Verified that the loading animation works correctly across the UI.

This ensures that the spinner rotates while the OWASP logo remains static in the center, complying with OWASP branding guidelines.

Preview

Screencast.from.2026-04-06.01-16-47.webm

Checklist

  • Required: I followed the contributing workflow
  • Required: I verified that my code works as intended and resolves the issue as described
  • Required: I ran make check-test locally: all warnings addressed, tests passed
  • I used AI for code, documentation, tests, or communication related to this PR

@coderabbitai

coderabbitai Bot commented Apr 5, 2026 •

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b052a59e-54f1-4dab-b4fe-af1c525105f8

📥 Commits

Reviewing files that changed from the base of the PR and between 3160c60 and 52f653c.

📒 Files selected for processing (2)
  • frontend/__tests__/unit/components/LoadingSpinner.test.tsx
  • frontend/src/components/LoadingSpinner.tsx

Summary by CodeRabbit

Release Notes

  • Tests

    • Added unit tests for the loading spinner component to ensure correct animation behavior and visual stability.
  • Style

    • Improved internal structure of the loading spinner component for better maintainability.

Walkthrough

The LoadingSpinner component is refactored to separate the animated ring from static content by introducing a two-layer container structure. A test is added to verify the spinning animation applies only to the ring while logo images remain static and are not descendants of the animated element.

Changes

Cohort / File(s) Summary
LoadingSpinner Component
frontend/src/components/LoadingSpinner.tsx
Refactors markup from a single div to a two-layer structure: outer container with relative positioning and inner animated spinner with absolute positioning (inset-0). Preserves animation and border styling while enabling static overlay content.
LoadingSpinner Tests
frontend/__tests__/unit/components/LoadingSpinner.test.tsx
Adds test assertions verifying the .animate-custom-spin class is present on the spinner element and that images with alt text "Loading indicator" are not descendants of the animated element.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested labels

frontend, frontend-tests

Suggested reviewers

  • kasya
  • arkid15r
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: fixing the loader to comply with OWASP branding guidelines by keeping the logo static during animation.
Description check ✅ Passed The description is directly related to the changeset, clearly explaining the branding issue, the implementation approach, and verification steps.
Linked Issues check ✅ Passed The code changes successfully separate the spinner ring from the OWASP logo, apply rotation only to the ring, and maintain the logo static, directly resolving issue #4338's requirements for OWASP branding compliance.
Out of Scope Changes check ✅ Passed All changes (LoadingSpinner component refactoring and corresponding unit test) are directly scoped to the OWASP logo branding compliance objective; no extraneous modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No issues found across 2 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@Saurabh-2607
Saurabh-2607 marked this pull request as ready for review April 5, 2026 20:47
@arkid15r
arkid15r enabled auto-merge April 11, 2026 01:41

@arkid15r arkid15r left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

Thanks for improving that 👍

@sonarqubecloud

Copy link
Copy Markdown

@codecov

codecov Bot commented Apr 11, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.05%. Comparing base (158dbae) to head (c3d9c1a).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4508   +/-   ##
=======================================
  Coverage   99.05%   99.05%           
=======================================
  Files         527      527           
  Lines       16882    16882           
  Branches     2372     2320   -52     
=======================================
  Hits        16723    16723           
  Misses         91       91           
  Partials       68       68           
Flag Coverage Δ
backend 99.50% <ø> (ø)
frontend 97.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
frontend/src/components/LoadingSpinner.tsx 100.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 158dbae...c3d9c1a. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@arkid15r
arkid15r added this pull request to the merge queue Apr 11, 2026
Merged via the queue into OWASP:main with commit 68911df Apr 11, 2026
38 checks passed
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.

OWASP Logo Rotates with Loading Spinner, Violating Branding Guidelines

2 participants