The font for navigation (346kb) is being preloaded regardless if the navigation bar is visible. In the case below it's being loaded for anonymous users.

CommentFileSizeAuthor
#11 authenticated.png250.43 KBmherchel
#11 anonymous.png184.09 KBmherchel
#9 3d2bf10ec2a74e45e4f4f4552ef818ba.png82.38 KBfinnsky
#8 Drupal-11.png182 KBkanchan bhogade
Home___Navigation.png568.86 KBmherchel

Issue fork drupal-3443915

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mherchel created an issue. See original summary.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Priority: Normal » Major

raising to major since it interferes with the normal use of the site.

We should be looking at potentially splitting the font up to reduce size see https://github.com/zachleat/glyphhanger

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

We should be looking at potentially splitting the font up to reduce size

💯💯💯

plopesc made their first commit to this issue’s fork.

plopesc opened merge request !7821

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Active » Needs review

Good catch 💯

Added logic to include the font if only the toolbar is being rendered.

About font split. Do you think it has to be addressed here? Or might be moved to a lower priority follow up?

finnsky’s picture

finnsky commented

RE
#3
#3

We need to check it first:
https://github.com/zachleat/glyphhanger/issues/22

To be sure that variable fonts supported. So i guess better to do it in separated ticket.

kanchan bhogade’s picture

kanchan bhogade commented
StatusFileSize
new Drupal-11.png182 KB

Hi,
I've tried to reproduce the issue on Drupal version 11.x
Please let me know if I've missed anything while reproducing the issue

Testing steps:

  1. Install Drupal
  2. Install Navigation and Navigation Top bar Module
  3. and checked with an Anonymous user

Adding screenshot

finnsky’s picture

finnsky commented
StatusFileSize
new 3d2bf10ec2a74e45e4f4f4552ef818ba.png82.38 KB

Just checked in last tugboat. Seems all fine

@Kanchan Bhogade probably cache?

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

About font split. Do you think it has to be addressed here? Or might be moved to a lower priority follow up?

Definitely a followup. Probably needs a bit of discussion and maybe even testing.

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new anonymous.png184.09 KB
new authenticated.png250.43 KB

Thanks for working on this! Looks to be working as expected now. In addition to the administrator/anonymous screenshots below, I also created an authenticated user that doesn't have access to use the navigation bar and made sure the font wasn't loading for them.

Looks great!

mherchel’s picture

mherchel
he/him
Primary language English
Location Gainesville, FL, US
commented

Opened followup: #3444136: Navigation: Discuss/implement splitting variable font

catch’s picture

catch
he/him
Primary language English
commented

+1 on just moving the #attached['html_head_link'] this way, makes loads of sense.

catch’s picture

catch
he/him
Primary language English
commented
Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Committed/pushed to 11.x, thanks! Moving to 10.3.x for backport with the rest of the navigation commits.

  • catch committed 3ac65a2e on 11.x 
    Issue #3443915 by plopesc, mherchel: Font for navigation is preloaded in...

  • catch committed 993644f5 on 10.3.x 
    Issue #3443915 by plopesc, mherchel: Font for navigation is preloaded in...
catch’s picture

catch
he/him
Primary language English
commented
Status: Patch (to be ported) » Fixed

  • catch committed 3ac65a2e on 11.0.x 
    Issue #3443915 by plopesc, mherchel: Font for navigation is preloaded in...

Status: Fixed » Closed (fixed)

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