Problem

After upgrade to 8.x-2.4, the toolbar icon shows smaller in most of the themes except the admin theme.
In the 8.x-2.3 version, we added the box-sizing attributes to the tools.css except for the 8.x-2.4 version.

Steps to reproduce

  1. install drupal
  2. install admin_toolbar: 2.4
  3. enable admin toolbar (admin_toolbar) and
    Admin Toolbar Extra Tools (admin_toolbar_tools), the latter provides the logo icon that shows the bug.
  4. see normal size logo icon.
  5. install bootstrap4
  6. go to appearance and set bootstrap as default theme.
  7. go to front page and see small sized logo icon

admin toolbar icon screenshot

CommentFileSizeAuthor
#10 admin_toolbar_2.4_border-box_css.png13.99 KBg-brodiei
#10 admin_toolbar_2.4_border-box.png7.03 KBg-brodiei
#10 admin_toolbar_2.4_mobile.png8.47 KBg-brodiei
#10 admin_toolbar_2.4.png12.96 KBg-brodiei
#10 admin_toolbar_2.3.png11.53 KBg-brodiei
#9 2020-10-17 at 2.54 PM.png12.15 KBcobenash
#9 2020-10-17 at 2.53 PM.png15.04 KBcobenash
#9 2020-10-17 at 2.52 PM.png22.91 KBcobenash
#2 admin_toolbar-modify-css-3177269-2-8.patch419 bytescobenash
2020-10-16 at 1.47 PM.png9.23 KBcobenash

Comments

cobenash created an issue. See original summary.

cobenash’s picture

cobenash
Location Taipei
commented
StatusFileSize
new admin_toolbar-modify-css-3177269-2-8.patch419 bytes

Provided patch to fix this.

cobenash’s picture

cobenash
Location Taipei
commented
cobenash’s picture

cobenash
Location Taipei
commented
Issue summary: View changes
cobenash’s picture

cobenash
Location Taipei
commented
Status: Active » Needs review
adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented

I'm not able to reproduce the problem in Drupal8, Drupal9.
Can you add t he steps to reproduce?

adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented

What theme are you using?

cobenash’s picture

cobenash
Location Taipei
commented

@adriancid

I use the bootstrap_sass theme. I think the problem will occur when we use most of the RWD theme.

Most of the themes will add the following attributes to keep the RWD layout.

*,
*::before,
*::after {
  box-sizing: border-box; }

That's the reason I think we should add this as default, and basically, it's the same attributes as the Drupal 9 logo icon.

The following code is for drupal 9.

.toolbar-icon-9 .toolbar-icon-admin-toolbar-tools-help:before {
  box-sizing: content-box;
  background-image: url(../misc/icons/ffffff/drupal-9-logo.svg);
  padding-bottom: 0;
  padding-left: 2px;
  padding-right: 2px;
  padding-top: 2px;
  margin-left: 4px;
}
cobenash’s picture

cobenash
Location Taipei
commented
StatusFileSize
new 2020-10-17 at 2.52 PM.png22.91 KB
new 2020-10-17 at 2.53 PM.png15.04 KB
new 2020-10-17 at 2.54 PM.png12.15 KB

@adriancid
I think the following steps are much clear. :)

Steps to Reproduce:
1. Use the admin toolbar 8.x-2.4
2. Use Bootstrap Theme or include any bootstrap css library.

Before add the bootstrap library.
before add bootstrap cdn

After add the bootstrap library.
after load CDN

The bootstrap theme add " box-sizing: border-box;" in the bootstrap css library.
Here is the code link: https://github.com/twbs/bootstrap/blob/main/scss/_reboot.scss
Line 19.
Bootstrap

g-brodiei’s picture

g-brodiei
Primary language Chinese, Traditional
commented
Status: Needs review » Needs work
StatusFileSize
new admin_toolbar_2.3.png11.53 KB
new admin_toolbar_2.4.png12.96 KB
new admin_toolbar_2.4_mobile.png8.47 KB
new admin_toolbar_2.4_border-box.png7.03 KB
new admin_toolbar_2.4_border-box_css.png13.99 KB

After reproducing, this is what I'd found.

The box-sizing property is only missing in 2.4 version. I believe this should be set as "border-box" to accommodate the sizing under smaller screens. This is what it appears to be if not set, see large logo.

While on 2.3 version, the box-sizing property was set to "content-box" (guess it was removed from 2.4 since this is the default value for this property). content-box does not include the border and padding size, which the logo icon will remain with 20px that exceeds the container height.

See fix with box-sizing:border-box on 2.4.

See screen shot for css styling of 2.3 against 2.4 comparison.
ver. 2.3
2.3

ver.2.4
2.4

Setting back to "needs work" so we allow admin_toolbar to fit with rwd settings.

cobenash’s picture

cobenash
Location Taipei
commented
g-brodiei’s picture

g-brodiei
Primary language Chinese, Traditional
commented
Issue summary: View changes
Status: Needs work » Needs review

Sorry, I misunderstood the description of this bug. Allow me to re-explain my discovery. In short, the behaviour of logo icon size shrinking does happen on 2.4, not 2.3 when applying bootstrap themes (not as an admin theme).

It seems that admin_toolbar has set box-sizing: "content-box" just to maintain icon's size by default on 2.3, and was removed on 2.4, thus the icon's size aspect is overridden by other responsive themes like bootstrap when they apply box-sizing: border-box.

Is the removal of box-sizing: content-box; in ver. 2.4 made by purpose??

If we want it to remain the same spec for logo icon as ver. 2.3. Then this patch applies perfectly.
If not and it was done on purpose, then this issue should be set to (work as design).

Updated issue summary for steps to reproduce.
Setting back to need review.

idebr’s picture

idebr commented
Version: 8.x-2.4 » 8.x-2.x-dev
Assigned: cobenash » Unassigned
Status: Needs review » Reviewed & tested by the community

This change was introduced in #3158374: Show Drupal Icon depending on which drupal major version it is.. It was not a deliberate design choice, but rather an oversight when copy/pasting CSS.

skaught’s picture

skaught
Primary language English
commented
Parent issue: » #3158374: Show Drupal Icon depending on which drupal major version it is.
skaught’s picture

skaught
Primary language English
commented

Is the removal of box-sizing: content-box; in ver. 2.4 made by purpose??

^^ in terms of original work from #3158374: Show Drupal Icon depending on which drupal major version it is. -- if I removed this is was an oversight. I am not familiar more directly with this modules lifespan to know about any other related changes.

also:
is this just thru bootstrap themes? will this be over compensate with any other general themes? is that being QA'd in anyway in this issue?

=====
edit/add-in:
Question: should this issue be open against the Bootstrap theme?

idebr’s picture

idebr commented

#15 This is not specific for bootstrap. See #3026302: Admin toolbar icon width/height changes for themes based on box-sizing: border-box where the box-sizing was introduced for more information.

  • adriancid committed 4926cda on 8.x-2.x authored by cobenash 
    Issue #3177269 by cobenash, g-brodiei: Missing box-sizing attributes on...
adriancid’s picture

adriancid
Primary language Spanish
Location Qc, Canada 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Thanks to @all?

g-brodiei’s picture

g-brodiei
Primary language Chinese, Traditional
commented

Thanks for providing all the relevant information!! Heaps of help knowing the context.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.