Problem/Motivation

  1. Toolbar adds an unnecessary top padding to body if the toolbar itself is unorinted (below 16.5em viewport width)
  2. Besides this, in Claro theme we want to change toolbar item's height a bit between out tablet and desktop layout, and we need toolbar to re-calculate it's height.
  3. There are cases when items of the toolbar bar break in more than one line. In these cases, the bar may overlap page content (including interactive elements as well).

Proposed resolution

#1 and #2 should be fixed by this FR.
#3 should be covered by #2958478: Toolbar height calculation is faulty in multiple cases

Remaining tasks

Patch.

User interface changes

The document's body top padding is calculated from the toolbar's bar height and the padding is re-calculated every time when the viewport changes. For extra small viewports body offset remains 0.

API changes

Hopefully nothing.

Data model changes

Nothing.

Comments

huzooka created an issue. See original summary.

huzooka’s picture

huzooka’s picture

Assigned: Unassigned » huzooka
huzooka’s picture

Assigned: huzooka » Unassigned
Status: Active » Needs review
Issue tags: +Needs subsystem maintainer review
StatusFileSize
new2.4 KB

Patch addresses #1, #2 and #3.

mcannon’s picture

@huzooka,
I want to fully test this out, but I think issue 1 needs more exact details about how to reproduce. "Toolbar adds an unnecessary top padding to body if the toolbar itself is unorinted (below 16.5em viewport width)". viewport width 16.5em isn't an exact number & needs detail about how to reproduce. Maybe just a screen shot of the issue here would be best.

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

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

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). 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: 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.

nod_’s picture

Status: Needs review » Needs work

Does fix the issue with padding on very small viewports.

Need to replace :visible by something else, we don't want jQuery specific selectors. or at least move it outside the main selector and add .filter(':visible').

Should use the getter to access isOriented value, any reasons to access it directly?

I have an issue with calling updateheight from adjustPlacement.
We have update height => trigger offset update => offest update trigger adjustPlacement

Actually I'm not quite sure why we're computing the height from the toolbar. There is code in Drupal.displace that does the job already. Doing this fixes the bug too:

      // Trigger a recalculation of viewport displacing elements. Use setTimeout
      // to ensure this recalculation happens after changes to visual elements
      // have processed.
      triggerDisplace(propagate) {
        _.defer(() => {
          $('body').css({
            'padding-top': Drupal.displace(propagate).top,
          });
        });
      },


// later in adjustPlacement:

        this.triggerDisplace(false);
nod_’s picture

Removing all the code for the height computation and keeping only the call to triggerDisplace works too. Might break something but it's not obvious.

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.

nod_’s picture

Issue tags: +JavaScript
droplet’s picture

Take HEIGHT from `.toolbar-tab` but not their parents prevent the flickering. (Although it has broken by other commits again very quickly after my fixes)

So that, adding `'change:mqMatches'` to the below line is better (if we only want to trigger `updateToolbarHeight`):
https://git.drupalcode.org/project/drupal/-/blob/9.2.x/core/modules/tool...

Virtually, you will fall into the above conditions, so I bet this issue has another problem I overlooked? eg. multiple lines on another issue? (I loaded Claro theme and can't see any special changes required in this patch)

EDIT:
BTW, To fix these kinds of Toolbar issues, we must define the default behaviors of Toolbar. Otherwise, the fixes are non-stop.

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.

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.

qusai taha’s picture

StatusFileSize
new2.48 KB

Re-roll for 9.4

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.

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.

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

Status: Needs work » Postponed
Issue tags: -JavaScript +JavaScript

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.