Consider a more substantial shortcuts placeholder

Created on 11 December 2024, 15 days ago

Problem/Motivation

The shortcuts links are rendered via a placeholder (and hence bigpipe), which is good.

However due to the &nsbbp; placeholder the parent link pops in after the page is loaded. Because it is generally quite high above the fold, this can be quite obvious.

Steps to reproduce

Proposed resolution

Can we just render the shortcuts top level link as a placeholder? This doesn't need any special caching and it means there is no visual effect when the placeholder/lazy builder is replaced.

Remaining tasks

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

📌 Task
Status

Active

Version

11.0 🔥

Component

navigation.module

Created by

🇬🇧United Kingdom catch

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @catch
  • 🇬🇧United Kingdom catch
  • Merge request !10532Add a more substantial preview. → (Open) created by catch
  • 🇬🇧United Kingdom catch

    Not a direct result, but this was found while working on 📌 Add performance testing Active .

  • First commit to issue fork.
  • Pipeline finished with Failed
    14 days ago
    Total: 91s
    #366341
  • Pipeline finished with Success
    14 days ago
    Total: 421s
    #366343
  • 🇪🇸Spain plopesc Valladolid

    After checking the MR I realized the resultant code was very similar to the original one we had in Navigation before merging it into core.

    There were concerns about this approach expressed in a MR comment that were addressed in the following commit.

    Are this concerns still valid and should we think in a different approach for this MR?

  • 🇬🇧United Kingdom catch

    So this is what I was worried about in that MR comment:

      '#lazy_builder' => ['navigation.user_lazy_builder:renderNavigationLinks', []],
          '#create_placeholder' => TRUE,
          '#cache' => [
            'keys' => ['user_set_navigation_links'],
            'contexts' => ['user'],
          ],
    
    

    e.g. the cache context for the placeholder itself was user-specific (I think to show the username). In the shortcuts case, we can always show 'Shortcuts' there, so it's the same for everyone and won't affect cache granularity.

    I did have a vague memory of that MR too and kept thinking I was missing something here while working on this, but so far it seems like for the shortcuts link it's always going to be the same for everyone. This might need some before/after manual testing when a user doesn't have access to shortcuts but does have access to navigation, or when their shortcuts are completely empty? But from reading the code if there's an issue with that it might be pre-existing.

Production build 0.71.5 2024