Skip to content

Conversation

@ymahtat-dev
Copy link

@ymahtat-dev ymahtat-dev commented Aug 5, 2025

Regarding the logo replacement activity in MOSIP-42060, we’ve reviewed all UI areas where the MOSIP logo is present,
Changes in Admin UI :

  • Normal screen : Logo Picto Only - ✅ Replaced (preferred due to space constraints and the presence of the “MOSIP + Admin UI version” text)
  • Medium width screen : Logo White (Full) – ✅ Replaced (preferred as text is hidden)
  • Small screen : Logo Picto Only - ✅ Replaced

Already merged in develop in PR #653

Signed-off-by: Youssef MAHTAT <youssef.mahtat.as.developer@gmail.com>
<img *ngIf="(screenResize > 840) || (screenResize <= 470)"
src="./assets/images/mosip-logo-picto.png"
style="max-height: 2.82rem; width: fit-content;"
alt="Logo"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

@CodeRabbit please check if logo changes are done properly and not breaking the ui

Copy link

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

echo "=== Checking if new logo files exist ==="
fd -t f "mosip-logo-picto.png" .
fd -t f "mosip-logo-white.png" .

echo ""
echo "=== Checking if old logo file exists ==="
fd -t f "logo-final.png" .

echo ""
echo "=== Checking other references to these logo files ==="
echo "--- References to mosip-logo-picto.png ---"
rg -n "mosip-logo-picto\.png" --type-add 'web:*.{html,ts,css,scss}' --type web

echo ""
echo "--- References to mosip-logo-white.png ---"
rg -n "mosip-logo-white\.png" --type-add 'web:*.{html,ts,css,scss}' --type web

echo ""
echo "--- References to logo-final.png (old logo) ---"
rg -n "logo-final\.png" --type-add 'web:*.{html,ts,css,scss}' --type web

echo ""
echo "=== Checking header component CSS/SCSS files ==="
fd -t f "header.component" -e css -e scss

Length of output: 1153


🏁 Script executed:

#!/bin/bash

echo "=== Checking header component SCSS for logo-related styles ==="
cat admin-ui/src/app/core/header/header.component.scss

echo ""
echo "=== Checking header component TypeScript for screenResize logic ==="
cat admin-ui/src/app/core/header/header.component.ts

Length of output: 4554


@dhanendra06 I've reviewed the logo changes and found several potential issues that could break the UI:

✅ What's Working

  1. Logo files exist: Both mosip-logo-picto.png and mosip-logo-white.png are present in the assets folder
  2. Conditional logic is complete: The screen size conditions cover all ranges without gaps:
    • screenResize > 840: picto logo
    • 470 < screenResize ≤ 840: white logo
    • screenResize ≤ 470: picto logo
  3. Old logo removed: No references to logo-final.png remain

⚠️ Potential UI Breaking Issues

1. Logo Size Conflict with CSS

The inline styles specify 2.82rem (~45px) height, but the SCSS file at Line 32-36 defines:

img {
  max-width: 100%;
  max-height: 100%;
  width: 35px;
  height: 35px;
}

While inline styles will override this, the toolbar height is only 35px (Line 1-8 in SCSS), which cannot accommodate a 45px tall logo.

2. Inconsistent Height Properties

  • Picto logo uses max-height: 2.82rem
  • White logo uses height: 2.82rem

This inconsistency could cause different rendering behavior between the logos.

3. Recommendation

Either:

  • Option A: Update the logo dimensions to 2.19rem (~35px) to match the toolbar height
  • Option B: Increase the toolbar height in the SCSS to accommodate the larger logos

Suggested fix for Option A:

<img *ngIf="(screenResize > 840) || (screenResize <= 470)"
     src="./assets/images/mosip-logo-picto.png"
     style="height: 2.19rem; width: fit-content;"
     alt="Logo"/>
<img *ngIf="(screenResize <= 840 && screenResize > 470)"
     src="./assets/images/mosip-logo-white.png"
     style="height: 2.19rem; width: fit-content;"
     alt="Logo"/>

Would you like me to help verify this in a test environment, or do you have specific screen sizes you'd like me to analyze further?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants