Problem/Motivation

When BigPipe is enabled the horizontal toolbar height is not calculated correctly for BigPipe-loaded menu trays. For example, when on the 'Edit profile' page with the user menu tray opened the top of page is partially obscured by the toolbar. See screenshot.

Proposed resolution

Since the toolbar height is calculated before BigPipe inserts additional menu trays, the only solution I could think of is triggering an event for the updateToolbarHeight() function to re-run on the menu tray(s) insertion.

Apologies in advance if this is a duplicate. I couldn't seem to find an issue for this specific problem.

CommentFileSizeAuthor
#25 2952447-nr-bot.txt144 bytesneeds-review-queue-bot
#21 2952447-21.patch2.38 KBravi.shankar
#12 toolbar_initial_height-2952447-12.patch2.28 KBjamesashok
#12 After Patch.png46.38 KBjamesashok
#12 Before Patch.png47.2 KBjamesashok
#8 interdiff-4-8.txt2.18 KBndf
#8 toolbar-incorrect-height-for-bigpipe-loaded-menus-2952447-8.patch3.46 KBndf
#4 interdiff-2952447-3-4-do-not-test.diff1.36 KBdaniel korte
#4 toolbar-incorrect-height-for-bigpipe-loaded-menus-2952447-4.patch3.4 KBdaniel korte
#3 toolbar-incorrect-height-for-bigpipe-loaded-menus-2952447-3.patch3.23 KBdaniel korte
#2 Screen Shot 2018-03-13 at 12.26.58 AM.png17.61 KBdaniel korte

Comments

Daniel Korte created an issue. See original summary.

daniel korte’s picture

daniel korte
he/him/his
Primary language English
Location Brooklyn, NY
commented
StatusFileSize
new Screen Shot 2018-03-13 at 12.26.58 AM.png17.61 KB
daniel korte’s picture

daniel korte
he/him/his
Primary language English
Location Brooklyn, NY
commented
Assigned: daniel korte » Unassigned
Status: Active » Needs review
StatusFileSize
new toolbar-incorrect-height-for-bigpipe-loaded-menus-2952447-3.patch3.23 KB

The attached patch adds an event in order to only trigger the updateToolbarHeight() function when a new .toolbar-menu is loaded by BigPipe.

daniel korte’s picture

daniel korte
he/him/his
Primary language English
Location Brooklyn, NY
commented
StatusFileSize
new toolbar-incorrect-height-for-bigpipe-loaded-menus-2952447-4.patch3.4 KB
new interdiff-2952447-3-4-do-not-test.diff1.36 KB

I noticed .toolbar-menu is not always wrapped in a div element. Updated for both scenarios.

chi’s picture

chi commented
Related issues: +#2951268: Improve rendering account link in the toolbar
borisson_’s picture

borisson_
Primary language Dutch
Location Mechelen, 🇧🇪
commented

I think we can close this issue as a duplicate of the issue linked in #5.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

ndf’s picture

ndf commented
Status: Needs review » Closed (outdated)
StatusFileSize
new toolbar-incorrect-height-for-bigpipe-loaded-menus-2952447-8.patch3.46 KB
new interdiff-4-8.txt2.18 KB

First a re-roll of #4.

I manually test this patch with 8.7.x, but I cannot see a difference.

Vaguely I do remember I noticed the behaviour from #2 in earlier Drupal Core versions.
But currently this behaviour is not there anymore.

This issue is not a duplicate of #2951268: Improve rendering account link in the toolbar. That issue has to do with the height of the icons in the toolbar while rendering.
This issue is about pushing the content-area down, when the optional part of the toolbar is expended horizontally.

Put this on closed (outdated) now.

Daniel Korte please re-open if I missed something here!

ndf’s picture

ndf commented

To be extra clear:
This issue is about pushing the content-area down, when the optional part of the toolbar is expended horizontally.
The bug from #2 does not exist anymore. So the content-area will not stay behind the toolbar and will be pushed down or up when opening/closing the toolbar.

But pushing the content-area down happens after the initial page render. That is issue #2998451: Toolbar tray rendering can result "flickering" resizing of content area to accommodate open trays

daniel korte’s picture

daniel korte
he/him/his
Primary language English
Location Brooklyn, NY
commented
Status: Closed (outdated) » Needs review

I am still seeing this issue in 8.7.7 and the latest patch still fixes it for me.

To reproduce the issue BigPipe must be enabled. After that, this issue only presents itself when the User menu item is selected and the horizontal tray is open. Refreshing the page with the User menu item selected and horizontal tray should obscure the page content. See screenshot:

Toolbar partially obscuring page content

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.9 was released on November 6 and is the final full bugfix release for the Drupal 8.7.x series. Drupal 8.7.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.8.0 on December 4, 2019. (Drupal 8.8.0-beta1 is available for testing.)

Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

jamesashok’s picture

jamesashok commented
Assigned: Unassigned » jamesashok
Issue tags: +toolbar, +overlap, +content, +updateToolbarHeight, +height, +ToolbarVisualView
StatusFileSize
new Before Patch.png47.2 KB
new After Patch.png46.38 KB
new toolbar_initial_height-2952447-12.patch2.28 KB

Issue is related to Issue #3021147.

The following patch is being updated on both issues.

The issue arises because the Toolbar height is being calculated before its contents are rendered on the initial load at ToolbarVisualView.js; so, the patch makes the height calculation asynchronous, by deferring it and pushing it to queue. So, once the contents are rendered, the height of the toolbar will be calculated.

Tested across all major browsers and OS simulations and adding a couple of screenshots..

Before Patch

Before Patch

After Patch

After Patch

toolbar_initial_height-2952447-12.patch

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: -toolbar, -overlap, -content, -updateToolbarHeight, -height, -ToolbarVisualView
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 8.8.x-dev » 8.9.x-dev
jamesashok’s picture

jamesashok commented
Assigned: jamesashok » Unassigned

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Needs work
Issue tags: +Needs reroll, +Bug Smash Initiative

Ran across this and found this existing issue. The same bug is there on 9.3 as descriped, patch doesn't apply any more though.

ravi.shankar’s picture

ravi.shankar commented
Issue tags: -Needs reroll
StatusFileSize
new 2952447-21.patch2.38 KB

Added reroll of patch #12.

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location Pune(MH), INDIA 🇮🇳
commented
Status: Needs work » Needs review

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 2952447-nr-bot.txt144 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

manuel.adan’s picture

manuel.adan
Location 🌌
commented
Related issues: +#3021147: Toolbar overlap content with big pipe enabled

Duplicate of #3021147: Toolbar overlap content with big pipe enabled ?

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

quietone’s picture

quietone commented
Status: Needs work » Postponed

The Toolbar Module was approved for removal in #3476882: [Policy] Move Toolbar module to contrib.

This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3484850: [meta] Tasks to deprecate Toolbar module and the removal work in #3488828: [meta] Tasks to remove Toolbar module.

Toolbar will be moved to a contributed project before Drupal 12.0.0 is released.

quietone’s picture

quietone commented
Project: Drupal core » Toolbar
Version: main » 1.x-dev
Component: toolbar.module » Code
Status: Postponed » Needs work

The Toolbar module have moved from core to contrib.