beaverbuilder / feature-requests

Feature request board for Beaver Builder products.
26 stars 4 forks source link

Move topbar so it's just inside the header element to avoid CLS issues and visual 'jumps' when using optimization plugins #353

Open jas8522 opened 4 months ago

jas8522 commented 4 months ago

When using a fixed header AND topbar, each must have their top positioning set independently because they're currently separate elements - only related as siblings. This results in a few issues:

  1. Cumulative Layout Shift (CLS) issues when running Google PageSpeed. (They're quite small, but still there)
  2. The header often visually shows as layered below the topbar on the z-index for a split second prior to page fully loading
  3. If you're using an optimization plugins that delays JS execution, that layering effect can continue until interaction on the page (mouse move or click, keyboard press, etc)

Solution:

Move the call to FLTheme::top_bar(); out of header.php and into each of the nav-* templates such that it's loaded within <header class="fl-page-header"> making it a child of header and a sibling to .fl-page-header-wrap. (In nav-vertical* files, topbar is at the start of the file, remaining a sibling to header).

The result of this is that there's no CLS or optimization problems visually, AND the topbar automatically gains the advantage of the header already having been positioned - no more need for code to position both the header and the topbar independently in various scenarios (ex: adminbar, bb editor header, etc)

The CSS additions that accompany this change are as follows:

.fl-shrink .fl-page-header .fl-page-bar,
.fl-fixed-header .fl-page-header .fl-page-bar{
  position: static; /* Since it's now inside the fixed position header */
}
.fl-page-header .fl-page-header-row{ position: relative; }

/* Force override the BB JS set top value */
.fl-shrink .fl-page-header,
.fl-fixed-header .fl-page-header{
  top: 0 !important; 
}
.fl-shrink.admin-bar .fl-page-header,
.fl-fixed-header.admin-bar .fl-page-header {
  top: 32px !important; 
}
@media screen and (max-width: 768px){
  .fl-shrink.admin-bar .fl-page-header,
    .fl-fixed-header.admin-bar .fl-page-header {
        top: 46px !important;
    }
}
@media (max-width: 991px) {
  .fl-page-nav-toggle-icon.fl-page-nav-toggle-visible-medium-mobile {
    position: static;
  }
}

Alternatives attempted:

I've tried using raw JS (no jQuery) that loads directly below the header via hook, however that just gets delayed by optimization plugins and looks bad on-screen.

Additional

This change would also bring the theme's header into alignment with how the Beaver Themer header module does it; if you want a topbar you simply add a row at the top, but that row is still within the actual header.

To ensure the topbar still loads if the header is disabled, the topbar code in header.php could become:

    $header_layout  = FLTheme::get_setting( 'fl-header-layout' );
    $header_enabled = apply_filters( 'fl_header_enabled', true );

    if ( 'none' === $header_layout || !$header_enabled ) {
        do_action( 'fl_before_top_bar' );

        FLTheme::top_bar();

        do_action( 'fl_after_top_bar' );
    }

Or that code could be removed from header.php and class-fl-theme.php : header_layout() could add: else{ self::top_bar(); }