Skip to content

Add ShowcaseLayout for apps #9013

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 2 commits into from
Oct 4, 2023
Merged

Add ShowcaseLayout for apps #9013

merged 2 commits into from
Oct 4, 2023

Conversation

sunnyclimber456
Copy link
Contributor

@sunnyclimber456 sunnyclimber456 commented Oct 4, 2023 •
edited
Loading

What does this change?

This change adapts the ShowcaseLayout for apps. Generally, apps layouts differ from web in the following ways:

  • Apps do not show the web header and footer. Apps show their own footer
  • Apps do not show web ads. Apps show their own ads
  • Apps do not show the web onwards container
  • Apps do not render the "body end slot"
  • Apps do not show the most viewed in the footer
showcaselayout.mp4

Closes #8997

@sunnyclimber456 sunnyclimber456 added the run_chromatic Runs chromatic when label is applied label Oct 4, 2023
@sunnyclimber456 sunnyclimber456 requested a review from a team as a code owner October 4, 2023 09:41
@github-actions
Copy link

github-actions bot commented Oct 4, 2023 •
edited
Loading

Size Change: 0 B 🆕

Total Size: 0 B

compressed-size-action

@sunnyclimber456
Copy link
Contributor Author

https://github.com/guardian/mobile-apps-api/pull/2713 adds support to MAPI for articles which use the apps ShowcaseLayout.

@ioannakok
Copy link
Contributor

Is merchandising-high ad slot the same in DCAR?

PS. I really wanted to comment on the unchanged line but we'll have to wait till this one's done!

ioannakok
ioannakok reviewed Oct 4, 2023
View reviewed changes
abeddow91
abeddow91 reviewed Oct 4, 2023
View reviewed changes
@sndrs sndrs mentioned this pull request Oct 4, 2023
Merged
@sunnyclimber456
Copy link
Contributor Author

Is merchandising-high ad slot the same in DCAR?

PS. I really wanted to comment on the unchanged line but we'll have to wait till this one's done!

@ioannakok I don't think so- I think apps articles don't include any of the web ad slots. We only render that slot in ShowcaseLayout if renderAds === true. The condition for renderAds is renderingTarget === 'Web' && canRenderAds(article);, so merchandising-high wouldn't render for ads.

@sunnyclimber456 sunnyclimber456 merged commit 6b09453 into main Oct 4, 2023
@sunnyclimber456 sunnyclimber456 deleted the gb/dcar-showcase-layout branch October 4, 2023 14:58
This was referenced Oct 10, 2023
Merged
Closed
@ioannakok ioannakok mentioned this pull request Oct 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ioannakok ioannakok ioannakok approved these changes

@abeddow91 abeddow91 abeddow91 left review comments

Assignees
No one assigned
Labels
dotcom-rendering run_chromatic Runs chromatic when label is applied
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Showcase layout
3 participants
@sunnyclimber456 @ioannakok @abeddow91