Problem/Motivation

Split from #3437499: Use placeholdering for more blocks, if we add more pages from Umami, we get a better 'worst case scenario' for the current aggregation / library organisation to improve from.

Steps to reproduce

Proposed resolution

Visit an extra page during these two test methods. Because this loads a different, third, combination of libraries to the first two pages, it results in a different set of asset aggregates and therefore more files and overall bytes being downloaded.

Issues like #3437499: Use placeholdering for more blocks which attempt to reduce duplication between asset aggregates in this kind of scenario will then be able to show an improvement (or not) based on the existing test scenario.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3445285

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

catch created an issue. See original summary.

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review

catch opened merge request !7917

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Appears to have some failures.

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: +gander, +frontend performance
catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review

MR is green now.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Tests are all green so going to lean on that.

catch’s picture

catch
he/him
Primary language English
commented
Issue summary: View changes
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Looks to need a reroll

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

immaculatexavier opened merge request !8144

immaculatexavier’s picture

immaculatexavier commented
Status: Needs work » Needs review

Rerolled against 11.x

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community

@immaculatexavier the 8144 MR had merge conflicts still, did you actually rebase against origin/11.x? Going to hide the MR, have rebased the original one here.

catch changed the visibility of the branch 3445285-add-extra-page-request to hidden.

catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work

Bit more to do re-rolling the original branch - Umami CSS got smaller in the meantime.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Reviewed & tested by the community

Straightforward changes, just adjusting the new assertion numbers to match the improved situation after the responsive grid issue landed.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Needs review

Still one open thread on the MR regarding the chroot calls

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs reroll

Argh I'd missed that comment on the MR. Had temporarily applied #3445847: PHPUnit 10 behaves differently when invoked outside web root as a cherry-pick to get tests passing locally, committed that issue while I was working on this one (which meant it would have disappeared after rebase), but because it then got reverted again, the change was still in here. Removed those.

quietone’s picture

quietone commented

I read the IS, comments and the MR. Everything looks good to go here. There are no unanswered questions.

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

  • alexpott committed 506ccd4e on 11.0.x 
    Issue #3445285 by catch, longwave, larowlan, quietone: Add extra page...

  • alexpott committed 8654d2b8 on 11.x 
    Issue #3445285 by catch, longwave, larowlan, quietone: Add extra page...
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 506ccd4 and pushed to 11.0.x. Thanks!
Committed 8654d2b and pushed to 11.x. Thanks!

Didn't apply to 10.4.x so didn't work on seeing why as this is test only.

Status: Fixed » Closed (fixed)

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