Skip to content

Make GetMatchTabs an island - #4014

Merged
ioannakok merged 1 commit into
mainfrom
ioanna/match-tabs-island
Feb 16, 2022
Merged

ioannakok merged 1 commit into
mainfrom
ioanna/match-tabs-island

Conversation

@ioannakok

@ioannakok ioannakok commented Feb 16, 2022 •

Copy link
Copy Markdown
Contributor

Co-authored-by: Oliver Lloyd oliver.lloyd@freelancer.guardian.co.uk
Co-authored-by: Joshua Lieberman joshua.lieberman@guardian.co.uk

What does this change?

  • Turning GetMatchTabs component to an Island.
  • Fixes issue where match tabs were not showing in standard article.

Why?

  • Islands is our new hydration pattern

Before

image

After

image

Co-authored-by: Oliver Lloyd <oliver.lloyd@freelancer.guardian.co.uk>
Co-authored-by: Joshua Lieberman <joshua.lieberman@guardian.co.uk>
@github-actions

Copy link
Copy Markdown

Size Change: -34.4 kB (-3%)

Total Size: 1.31 MB

Filename Size Change
dotcom-rendering/dist/1924.js 5.04 kB +394 B (+8%) 🔍
dotcom-rendering/dist/1924.legacy.js 5.11 kB +403 B (+9%) 🔍
dotcom-rendering/dist/2.js 4.36 kB +414 B (+10%) ⚠️
dotcom-rendering/dist/2.legacy.js 4.47 kB +412 B (+10%) ⚠️
dotcom-rendering/dist/2058.js 5.78 kB +392 B (+7%) 🔍
dotcom-rendering/dist/2058.legacy.js 5.95 kB +402 B (+7%) 🔍
dotcom-rendering/dist/2393.js 3.82 kB +411 B (+12%) ⚠️
dotcom-rendering/dist/2393.legacy.js 3.97 kB +418 B (+12%) ⚠️
dotcom-rendering/dist/281.legacy.js 0 B -3.42 kB (removed) 🏆
dotcom-rendering/dist/3215.js 5.39 kB +394 B (+8%) 🔍
dotcom-rendering/dist/3215.legacy.js 5.52 kB +403 B (+8%) 🔍
dotcom-rendering/dist/4889.js 0 B -4.55 kB (removed) 🏆
dotcom-rendering/dist/4889.legacy.js 0 B -4.56 kB (removed) 🏆
dotcom-rendering/dist/5356.js 5.5 kB +391 B (+8%) 🔍
dotcom-rendering/dist/5356.legacy.js 5.65 kB +395 B (+8%) 🔍
dotcom-rendering/dist/5484.legacy.js 0 B -7.89 kB (removed) 🏆
dotcom-rendering/dist/7051.js 7.76 kB +403 B (+5%) 🔍
dotcom-rendering/dist/7051.legacy.js 7.91 kB +410 B (+5%) 🔍
dotcom-rendering/dist/8280.js 0 B -3.29 kB (removed) 🏆
dotcom-rendering/dist/8838.js 0 B -3.02 kB (removed) 🏆
dotcom-rendering/dist/8880.legacy.js 0 B -3.06 kB (removed) 🏆
dotcom-rendering/dist/9754.js 0 B -5.21 kB (removed) 🏆
dotcom-rendering/dist/9884.js 17 kB -410 B (-2%)
dotcom-rendering/dist/9884.legacy.js 17.3 kB -434 B (-2%)
dotcom-rendering/dist/frontend.server.js 352 kB +604 B (0%)
dotcom-rendering/dist/initDiscussion.js 7.9 kB +7 B (0%)
dotcom-rendering/dist/initDiscussion.legacy.js 8.15 kB +13 B (0%)
dotcom-rendering/dist/InteractiveBlockComponent.js 5.75 kB +2.72 kB (+90%) 🆘
dotcom-rendering/dist/InteractiveBlockComponent.legacy.js 5.96 kB +2.78 kB (+87%) 🆘
dotcom-rendering/dist/islands.js 8.07 kB +9 B (0%)
dotcom-rendering/dist/islands.legacy.js 8.8 kB +13 B (0%)
dotcom-rendering/dist/react.js 65.8 kB -7.58 kB (-10%) 👏
dotcom-rendering/dist/react.legacy.js 72.2 kB -7.7 kB (-10%) 👏
dotcom-rendering/dist/YoutubeBlockComponent.js 5.35 kB +2.45 kB (+85%) 🆘
dotcom-rendering/dist/YoutubeBlockComponent.legacy.js 5.53 kB +2.5 kB (+82%) 🆘
ℹ️ View Unchanged
Filename Size
dotcom-rendering/dist/1214.js 2.65 kB
dotcom-rendering/dist/1214.legacy.js 2.71 kB
dotcom-rendering/dist/1376.js 1.9 kB
dotcom-rendering/dist/1376.legacy.js 2.01 kB
dotcom-rendering/dist/1624.js 2.64 kB
dotcom-rendering/dist/1624.legacy.js 2.71 kB
dotcom-rendering/dist/2177.js 3.7 kB
dotcom-rendering/dist/262.js 5.61 kB
dotcom-rendering/dist/2748.js 9.11 kB
dotcom-rendering/dist/2748.legacy.js 9.59 kB
dotcom-rendering/dist/2879.js 9.01 kB
dotcom-rendering/dist/2879.legacy.js 9.56 kB
dotcom-rendering/dist/2947.js 2.58 kB
dotcom-rendering/dist/2947.legacy.js 2.66 kB
dotcom-rendering/dist/2949.js 4.92 kB
dotcom-rendering/dist/2949.legacy.js 5.11 kB
dotcom-rendering/dist/3213.js 13.5 kB
dotcom-rendering/dist/3213.legacy.js 13.8 kB
dotcom-rendering/dist/3249.js 3.42 kB
dotcom-rendering/dist/3397.js 240 B
dotcom-rendering/dist/3397.legacy.js 251 B
dotcom-rendering/dist/4005.js 30.9 kB
dotcom-rendering/dist/4025.js 3.98 kB
dotcom-rendering/dist/4025.legacy.js 4.31 kB
dotcom-rendering/dist/4211.js 2 kB
dotcom-rendering/dist/4211.legacy.js 2.11 kB
dotcom-rendering/dist/4576.js 235 B
dotcom-rendering/dist/4576.legacy.js 247 B
dotcom-rendering/dist/4813.js 5.29 kB
dotcom-rendering/dist/4813.legacy.js 5.39 kB
dotcom-rendering/dist/4850.js 235 B
dotcom-rendering/dist/4850.legacy.js 246 B
dotcom-rendering/dist/4999.js 18.4 kB
dotcom-rendering/dist/4999.legacy.js 19 kB
dotcom-rendering/dist/5096.js 4.67 kB
dotcom-rendering/dist/5096.legacy.js 4.74 kB
dotcom-rendering/dist/5217.js 233 B
dotcom-rendering/dist/5217.legacy.js 245 B
dotcom-rendering/dist/53.js 4.11 kB
dotcom-rendering/dist/53.legacy.js 4.12 kB
dotcom-rendering/dist/5585.js 6.47 kB
dotcom-rendering/dist/5585.legacy.js 6.72 kB
dotcom-rendering/dist/6046.js 6.45 kB
dotcom-rendering/dist/6046.legacy.js 6.83 kB
dotcom-rendering/dist/6222.legacy.js 3.47 kB
dotcom-rendering/dist/6348.js 5.12 kB
dotcom-rendering/dist/6348.legacy.js 5.29 kB
dotcom-rendering/dist/6400.js 21.5 kB
dotcom-rendering/dist/6400.legacy.js 21.5 kB
dotcom-rendering/dist/6684.js 241 B
dotcom-rendering/dist/6684.legacy.js 252 B
dotcom-rendering/dist/6854.legacy.js 28.9 kB
dotcom-rendering/dist/6965.js 3.19 kB
dotcom-rendering/dist/6965.legacy.js 3.25 kB
dotcom-rendering/dist/6992.js 2.02 kB
dotcom-rendering/dist/6992.legacy.js 2.12 kB
dotcom-rendering/dist/7642.legacy.js 8.31 kB
dotcom-rendering/dist/7671.legacy.js 3.83 kB
dotcom-rendering/dist/7800.js 11 kB
dotcom-rendering/dist/8080.js 1.81 kB
dotcom-rendering/dist/8080.legacy.js 1.9 kB
dotcom-rendering/dist/8129.legacy.js 11.5 kB
dotcom-rendering/dist/8294.js 232 B
dotcom-rendering/dist/8294.legacy.js 244 B
dotcom-rendering/dist/8748.js 232 B
dotcom-rendering/dist/8748.legacy.js 243 B
dotcom-rendering/dist/9327.js 1.85 kB
dotcom-rendering/dist/9327.legacy.js 1.95 kB
dotcom-rendering/dist/9641.js 6.44 kB
dotcom-rendering/dist/9641.legacy.js 6.7 kB
dotcom-rendering/dist/9776.js 6.37 kB
dotcom-rendering/dist/9776.legacy.js 6.63 kB
dotcom-rendering/dist/9817.js 11.5 kB
dotcom-rendering/dist/9817.legacy.js 11.7 kB
dotcom-rendering/dist/atomIframe.js 1.87 kB
dotcom-rendering/dist/atomIframe.legacy.js 2.14 kB
dotcom-rendering/dist/bootCmp.js 7.31 kB
dotcom-rendering/dist/bootCmp.legacy.js 10.9 kB
dotcom-rendering/dist/braze-web-sdk-core.js 36.1 kB
dotcom-rendering/dist/braze-web-sdk-core.legacy.js 36.1 kB
dotcom-rendering/dist/cmp.js 7.43 kB
dotcom-rendering/dist/coreVitals.js 4.03 kB
dotcom-rendering/dist/coreVitals.legacy.js 4.33 kB
dotcom-rendering/dist/dynamicImport.js 3 kB
dotcom-rendering/dist/dynamicImport.legacy.js 3.29 kB
dotcom-rendering/dist/embedIframe.js 1.88 kB
dotcom-rendering/dist/embedIframe.legacy.js 2.14 kB
dotcom-rendering/dist/ga.js 3.88 kB
dotcom-rendering/dist/ga.legacy.js 4.14 kB
dotcom-rendering/dist/guardian-braze-components-banner.js 11.9 kB
dotcom-rendering/dist/guardian-braze-components-banner.legacy.js 12 kB
dotcom-rendering/dist/guardian-braze-components-end-of-article.js 8.11 kB
dotcom-rendering/dist/guardian-braze-components-end-of-article.legacy.js 8.18 kB
dotcom-rendering/dist/newsletterEmbedIframe.js 2.03 kB
dotcom-rendering/dist/newsletterEmbedIframe.legacy.js 2.29 kB
dotcom-rendering/dist/ophan.js 7.18 kB
dotcom-rendering/dist/ophan.legacy.js 7.38 kB
dotcom-rendering/dist/readerRevenueDevUtils.js 892 B
dotcom-rendering/dist/readerRevenueDevUtils.legacy.js 952 B
dotcom-rendering/dist/relativeTime.js 2.41 kB
dotcom-rendering/dist/relativeTime.legacy.js 2.68 kB
dotcom-rendering/dist/sentry.js 677 B
dotcom-rendering/dist/sentry.legacy.js 689 B
dotcom-rendering/dist/sentryLoader.js 4.75 kB
dotcom-rendering/dist/sentryLoader.legacy.js 7.71 kB
dotcom-rendering/dist/shimport.js 2.75 kB
dotcom-rendering/dist/shimport.legacy.js 2.76 kB
dotcom-rendering/dist/SignInGateMain.js 3.56 kB
dotcom-rendering/dist/SignInGateMain.legacy.js 3.69 kB

compressed-size-action

@ioannakok
ioannakok marked this pull request as ready for review February 16, 2022 17:00
@ioannakok
ioannakok requested review from a team, JamieB-gu and marsavar as code owners February 16, 2022 17:00

@oliverlloyd oliverlloyd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep

@ioannakok
ioannakok merged commit 725f246 into main Feb 16, 2022
@ioannakok
ioannakok deleted the ioanna/match-tabs-island branch February 16, 2022 17:11
@mxdvl

mxdvl commented Feb 17, 2022

Copy link
Copy Markdown
Contributor

Can someone explain to me how this fixed the missing tab issue?

@ioannakok

ioannakok commented Feb 18, 2022 •

Copy link
Copy Markdown
Contributor Author

Can someone explain to me how this fixed the missing tab issue?

I had a look at this and actually that was not a prod issue at all, only a story issue.

HydratedLayout: https://github.com/guardian/dotcom-rendering/blob/main/dotcom-rendering/src/web/layouts/Standard.stories.tsx#L259 calls doStorybookHydration
https://github.com/guardian/dotcom-rendering/blob/main/dotcom-rendering/src/web/browser/islands/doStorybookHydration.js

And since there was no GetMatchTabs.importable.tsx the component was not getting hydrated.

@ioannakok

Copy link
Copy Markdown
Contributor Author

As a side note, I realised the set up of the Standard/MatchReport story is a bit weird: https://www.chromatic.com/component?buildNumber=10431&historyLengthAtIndex=9&distanceToMoveBack=-4&appId=5dfcbf3012392c0020e7140b&name=Layouts%2FStandard&specName=MatchReport&componentInspectorKey=620fc019f033a3003af5ed41-1300-interactive-true. MatchNav and MatchTabs come from the Leicester - Arsenal match and the rest of the article body from the Swansea - Norwich.

That is confusing: The Swansea - Norwich match report does not have MatchTabs because it does not have data in minByMinUrl. Condition to render MatchTabs here: https://github.com/guardian/dotcom-rendering/blob/main/dotcom-rendering/src/web/components/GetMatchTabs.importable.tsx#L26

Swansea - Norwich: https://www.theguardian.com/football/2021/feb/05/andre-ayew-sparks-swansea-victory-over-norwich-to-close-gap-at-top
Leicester - Arsenal: https://www.theguardian.com/football/2021/feb/28/leicester-arsenal-premier-league-match-report

@mxdvl

mxdvl commented Mar 25, 2022

Copy link
Copy Markdown
Contributor

🏝️ #3629

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants