Skip to content

Fix mobile banner alignment and height restrictions for Blabber Footer Start banners#40

Closed
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-d9ff7c8e-9d96-4dde-8ab9-f42857888d97
Closed

Fix mobile banner alignment and height restrictions for Blabber Footer Start banners#40
Copilot wants to merge 2 commits into
mainfrom
copilot/fix-d9ff7c8e-9d96-4dde-8ab9-f42857888d97

Conversation

Copilot AI commented Jul 16, 2025

Copy link
Copy Markdown

Problem

Blabber Footer Start banners had two critical issues on mobile devices:

  1. Height cut-off: Banners were restricted to 100px height on mobile, cutting off larger banners like 250x250 mobile formats
  2. Forced centering: All banners were forced to center on mobile regardless of their alignment setting (left, center, right)

Root Cause

The CSS media query for mobile devices (max-width: 768px) contained overly restrictive rules:

/* BEFORE: Problematic mobile CSS */
@media screen and (max-width: 768px) {
    .iwz-blabber-footer-banner {
        justify-content: center !important;  /* Forced ALL banners to center */
        text-align: center !important;
    }
    
    .iwz-blabber-footer-banner iframe {
        height: 250px !important;            /* Conflicted with 100px rule above */
        max-height: 250px !important;
    }
}

/* Earlier in file: Height restriction that overrode mobile settings */
.iwz-blabber-footer-banner iframe {
    height: 100px !important;
    max-height: 100px !important;          /* This took precedence */
}

Solution

1. Fixed Height Restrictions

Added dedicated mobile CSS override to allow flexible banner heights:

/* Allow flexible height for mobile devices */
@media screen and (max-width: 768px) {
    .iwz-blabber-footer-banner iframe {
        height: auto !important;
        max-height: 250px !important;
        min-height: 100px !important;
    }
}

2. Fixed Mobile Alignment

Completely rewrote mobile alignment logic to respect individual banner settings:

/* Left alignment on mobile (default and explicit) */
.iwz-blabber-footer-banner.align-left,
.iwz-blabber-footer-banner:not(.align-center):not(.align-right) {
    justify-content: flex-start !important;
    text-align: left !important;
}

/* Center alignment on mobile (only when explicitly set) */
.iwz-blabber-footer-banner.align-center {
    justify-content: center !important;
    text-align: center !important;
}

/* Right alignment on mobile */
.iwz-blabber-footer-banner.align-right {
    justify-content: flex-end !important;
    text-align: right !important;
}

Impact

Before Fix ❌

  • All banners forced to center on mobile regardless of alignment setting
  • Banner height limited to 100px, cutting off 250x250 mobile banners
  • Left/right alignment settings ignored on mobile

After Fix ✅

  • Left-aligned banners stay left on mobile
  • Center-aligned banners stay centered on mobile
  • Right-aligned banners stay right on mobile
  • Default behavior is left-aligned (not centered) for banners without explicit alignment
  • Banner height can be up to 250px on mobile, supporting standard mobile banner formats
  • Desktop behavior completely unchanged

Testing

Added comprehensive manual testing documentation to README.md including:

  • Banner size testing (250x250, 300x250, etc.)
  • Alignment verification across different viewport sizes
  • Browser compatibility testing guidelines
  • Responsive breakpoint validation

Backward Compatibility

Fully backward compatible

  • Desktop behavior (769px+) unchanged
  • Existing CSS class structure preserved
  • PHP integration points unmodified
  • Only mobile-specific CSS enhanced

Files Changed

  • public/css/iwz-banner-container-public.css - Core mobile CSS fixes
  • README.md - Added testing documentation and updated examples

This fix ensures that Blabber Footer Start banners display correctly on mobile devices while respecting user-configured alignment settings and supporting standard mobile banner dimensions.

Warning

Firewall rules blocked me from connecting to one or more addresses

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/PHPCSStandards/PHP_CodeSniffer/zipball/5b5e3821314f947dd040c70f7992a64eac89025c
    • Triggering command: /usr/bin/php8.3 -n -c /tmp/CxXwgs /usr/bin/composer install (http block)

If you need me to access, download, or install something from one of these locations, you can either:

This pull request was created as a result of the following prompt from Copilot chat.

Issue: Blabber Footer Start Banner for mobile gets cut off and/or not aligned properly

Problem summary:

  1. Banners such as iframes added to the "Blabber Footer Start (Just Above Footer Area)" are being cut off at the bottom on mobile devices (height/visibility issue).
  2. Left-aligned banners still appear centered on mobile, not left-aligned as expected.

Root cause and requirements:

  • The CSS currently forces all Blabber Footer Start banners to be centered on mobile (max-width: 768px), regardless of their alignment setting (left, center, right). Also, the iframe height is often fixed at 100px and max-width 100%, which may cut off banners taller than 100px (like 250x250 mobile banners).

What needs to be done:

  1. Fix vertical cut-off:
    • Allow banner iframes to have a greater height on mobile (e.g., 250px), especially for 250x250 banner formats. Make height flexible based on the banner's actual size, or at least support 250px height for mobile.
    • Ensure max-height rules do not restrict the banner height to 100px on mobile for Blabber Footer Start location.
  2. Respect alignment on mobile:
    • Make sure left, center, right alignment settings are respected on mobile. Only force centering for banners that are set to center, not for all banners.
    • Update mobile CSS to allow .align-left and .align-right to work as on desktop.
  3. Testing:
    • Test with various banner sizes (e.g., 250x250, 300x250, etc.) to confirm they are not cut off or misaligned on mobile.

Solution summary:

  • Update public/css/iwz-banner-container-public.css:
    • Adjust media queries for mobile to:
      • Remove or modify max-height: 100px for iframes in Blabber Footer Start banner on mobile.
      • Only center banners if they have .align-center, otherwise allow left/right alignment on mobile as on desktop.
      • Allow iframe height up to 250px or use height: auto with max-height: 100vw for aspect ratio safety.
  • Add visual regression test instructions in README if not present.

This PR will:

  • Fix mobile Blabber Footer Start banner cut-off by relaxing restrictive height CSS.
  • Restore left/right alignment options for mobile, not just center.
  • Add comments to CSS for future maintainers.
  • Suggest manual test scenarios in README.

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@jasperf

jasperf commented Jul 16, 2025

Copy link
Copy Markdown
Contributor

@copilot these suggestions do take into account that we set banners' alignment in the dashboard / on the settings page and so are dynamic ?

@jasperf jasperf requested a review from Copilot July 16, 2025 08:04

This comment was marked as outdated.

@jasperf jasperf marked this pull request as ready for review July 16, 2025 08:04
@jasperf jasperf requested a review from Copilot July 16, 2025 08:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

@jasperf jasperf closed this Jul 16, 2025
@jasperf jasperf reopened this Jul 16, 2025
…r Start banners

Co-authored-by: jasperf <344138+jasperf@users.noreply.github.com>
@jasperf jasperf closed this Jul 16, 2025
Copilot AI changed the title [WIP] Fix mobile Blabber Footer Start banner cut-off and alignment issues Fix mobile banner alignment and height restrictions for Blabber Footer Start banners Jul 16, 2025
Copilot AI requested a review from jasperf July 16, 2025 08:09
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.

3 participants