[7571] On the BanneredCampaignPage, only show root page title if the …

[richbrennan]

…menu is shown too

Closes #7571

This PR contains a change to the Banner Page template to only show the parent page title if the menu is shown:

• Page with menu, parent title shown: https://foundation-s-fix-7571-g-pex1xa.mofostaging.net/en/campaigns/multi-page/concern-letter-nor/
• Page without menu, pages own title shown: https://foundation-s-fix-7571-g-pex1xa.mofostaging.net/en/giving/happy-holidays-from-mozilla/
  • Edit: https://foundation-s-fix-7571-g-pex1xa.herokuapp.com/cms/pages/165/edit/

The menu is shown when one or more child pages of the parent have 'Promote' > 'Show in menus' ticked.

[github-actions]

This PR introduces visual differences. Click here to inspect the diffs.

[github-actions]

This PR introduces visual differences. Click here to inspect the diffs.

[kristinashu]

Design-wise, this is working well!

[Pomax]

Good find!

[Pomax]

Hm, one thing that percy flags: https://foundation-s-fix-7571-g-pex1xa.mofostaging.net/en/campaigns/initial-test-bannered-campaign-with-fixed-title/ no longer uses its title, but now uses its header instead.

(edit page: https://foundation-s-fix-7571-g-pex1xa.herokuapp.com/cms/pages/76/edit/)

@kristina for a banner page with both title and header, what should happen in this case?

[Pomax]

noticed something different happening on https://foundation-s-fix-7571-g-pex1xa.mofostaging.net/en/campaigns/initial-test-bannered-campaign-with-fixed-title/ now

[kristinashu]

Yikes, thanks for flagging that @Pomax! It should show the Title at the top of the page. TBH I don't really understand the differences in the Title and the Header.

[github-actions]

This PR introduces visual differences. Click here to inspect the diffs.

[richbrennan]

❤️ the visual regression tests.

There are some subtleties in the template logic but basically the header will override the title if its set.

This is complicated by an extra variable (root) that's being passed to the template, which is either the parent page if the parent is also a Banner page, or the page itself if the parent isn't a Banner page.

Before my change, the root variable title was being used as first priority, which suggests that the header will never be shown as root is always set to something 😕

I'm painted into a bit of a corner with regard to template logic unless I remove outputting the header, which doesn't seem to be shown anyway because of root. I've tried removing and re-deployed, and tested with the following pages:

• https://foundation-s-fix-7571-g-pex1xa.mofostaging.net/en/campaigns/initial-test-bannered-campaign-with-fixed-title/
• https://foundation-s-fix-7571-g-pex1xa.mofostaging.net/en/mozilla-insights/meet-the-team/
• https://foundation-s-fix-7571-g-pex1xa.mofostaging.net/en/giving/happy-holidays-from-mozilla/

[kristinashu]

Did this break multi-pages under /giving? I added a sub page to https://foundation-s-fix-7571-g-pex1xa.mofostaging.net/en/giving/happy-holidays-from-mozilla/ and it isn't showing up.

[Pomax]

@kristina I just did a test against main and it does the same thing: "giving" is a banner page itself, and so has 1 level of children that it will show in the nav. Children of children do not show up, so "giving"→"holiday"→"sub page" won't show. It will show "itself" as the "overview" entry, and "holidy" as second option in the nav.

[Pomax]

@richbrennan @kristinashu I'm okay with removing the header field if we're not using it, but then let's make sure we also test this against our current real data in the staging db. @richbrennan you should be able to run inv copy-stage-db locally for testing.

[richbrennan]

Thanks @Pomax

I am wary of introducing more regressions but I think I've made the smallest change I can to minimise them. I tested the following pages on staging vs a local staging backup and they all seem OK. I've attached a screenshot of a difference in menu where draft pages are shown locally, but I think its a feature not a bug.

@kristinashu Are you aware of any pages besides those below that might be affected by the change?

• https://foundation.mofostaging.net/en/what-we-fund/
• All pages under: https://foundation.mofostaging.net/en/giving/
• https://foundation.mofostaging.net/en/who-we-are/
• Pages under https://foundation.mofostaging.net/en/insights/
• All pages under: https://foundation.mofostaging.net/en/data-futures-lab/
• https://foundation.mofostaging.net/en/internet-health/
• https://mozillafestival.mofostaging.net/en/get-to-know-mozfest/
• https://mozillafestival.mofostaging.net/en/proposals/
• https://mozillafestival.mofostaging.net/en/on-demand/
• https://mozillafestival.mofostaging.net/en/sponsor/
• https://mozillafestival.mofostaging.net/en/newcomer-guide-to-mozfest/
• https://mozillafestival.mofostaging.net/en/technologists/

[kristinashu]

Thank you for the list Rich. So this update won't change anything on /campaigns pages? The original issue isn't happening on those pages so they don't need any update. I'm just checking because they are our most used pages.
E.g.
https://foundation.mofostaging.net/en/campaigns/election-integrity-and-platform-accountability/
https://foundation.mofostaging.net/en/campaigns/tell-amazon-connected-devices-must-have-privacy-policies/

[kristinashu]

Sorry, it's a can of worms! Re:

@kristina I just did a test against main and it does the same thing: "giving" is a banner page itself, and so has 1 level of children that it will show in the nav. Children of children do not show up, so "giving"→"holiday"→"sub page" won't show. It will show "itself" as the "overview" entry, and "holidy" as second option in the nav.

I just tested on staging and confirm the sub-page of "holiday" aren't showing up. That seems like a separate bug because it should show up (like it does on all other pages https://foundation.mofostaging.net/en/campaigns/meet-mozillas-advocacy-and-research-team/). This is where the "Header" text is used, although perhaps renaming it to "Name in sub-nav" or "Sub-nav name" would be more helpful. Would removing the Header field only happen if there are no sub-child pages?

tl;dr is that pages under /giving should behave the same as pages under /campaign pages

Also note that PNI is more important than this PR so if it is more complex than it appears it's ok to pause on it. Also, I hope I'm not derailing this too much. I don't fully understand all the implications.

[Pomax]

The difference is that "giving" is already a real, active banner page, which is the bannered version of a campaign page, whereas the campaigns part of the campaign URL you showed is not (specifically, it's an index listing page for anything that matches the campaign backend page type), so the real campaign there is "meet-mozilla-...".

Basically, in terms of "how does the page present to the user as content-with-menu", these two URLs take you to the same "type" of page:

1. https://foundation.mofostaging.net/en/giving
2. https://foundation.mofostaging.net/en/campaigns/meet-mozillas-advocacy-and-research-team

On both URLs, only direct children of "that page" show up in the menu, so:

1. all direct children of "giving" show in the menu. Children of those children do not.
2. all direct children of "meet-mozillas-advocacy-and-research-team" show in the menu. Children of those children do not.

[Pomax]

Having said that, https://foundation.mofostaging.net/en/giving no longer seems to show any content, despite the CMS clearly showing that there is (see https://foundation.mofostaging.net/cms/pages/1126 and https://foundation.mofostaging.net/cms/pages/1126/edit), so that's definitely a problem. @richbrennan it might make sense to put this PR on pause for a bit to see if we first write out the exact behaviour we want to have happening here (with @kristinashu's input/sign-off) before we then update the code to implement that behaviour.

[Pomax]

Let's pause this for a bit so we can formulate a plan of attack.

[richbrennan]

Page/template investigation

This investigation covers what is displayed in the H1 element on applicable pages as per the original issue. I haven't looked at the menus that use the header field as that's a different component, do we need to investigate these too, if so would a separate issue be less likely to cause confusion?

The title/heading logic that displays the H1 is contained in the primary_heroguts.html template. This is used by the following pages:

• Banner Page: bannered_campaign_page.html
• Homepage: homepage.html
• Primary Page: primary_page.html
• MozFest Homepage: mozfest_homepage.html
• MozFest Primary page: mozfest-base.html

The H1 is populated by these variables in the following priority order:

1. root title
2. page header
3. page title

root is a custom variable (an instance of a page) who's value is set depending on the page's parent type, see individual descriptions below.

Individual page logic

Homepage
The title/heading logic of the primary_heroguts.html template is not used on the homepage.

Banner Page
If any ancestor page (i.e. a page above this one in the page tree) is also a Banner page, the root variable is set to be the highest ancestor Banner page up in the page tree. This page's "What We Fund" ancestor's title will be shown in the H1: https://foundation.mofostaging.net/en/what-we-fund/fellowships/senior-fellows/
If there are no other Banner pages as ancestors then the root is set to the page itself, so the page's own title is shown in the H1.
I don't believe that the header field will ever be shown in the H1 on a Banner page, because its always overridden by the root variable (ancestor page or the actual page).

Primary Page
The Primary page uses the same logic as the Banner Page for determining the root variable and what to output in the H1.
Example Primary page with a Primary page ancestor, so root is set to the ancestor whos title is displayed in the H1: https://foundation.mofostaging.net/en/internet-health/trustworthy-artificial-intelligence/
Example Primary page without a Primary page ancestor, so root is set to the page itself and it's own title is output in the H1: https://foundation.mofostaging.net/en/press-center/

MozFest Homepage
The title/heading logic of the primary_heroguts.html template is not used on the MozFest homepage.

MozFest Primary Page
The MozFest Primary page uses the same logic as the Banner Page for determining the root variable and what to output in the H1.
Example MozFest Primary page with a MozFest Primary page ancestor, so root is set to the ancestor whos title is displayed in the H1: https://mozillafestival.mofostaging.net/en/on-demand/virtual-dialogue-and-debates/big-tech-and-germanys-election/
Example MozFest Primary page without a MozFest Primary page ancestor, so root is set to the page itself and it's own title is output in the H1 https://mozillafestival.mofostaging.net/en/technologists/ai-irl-hackathon-building-trustworthy-ai/

Questions/issues

Thank you for the list Rich. So this update won't change anything on /campaigns pages? The original issue isn't happening on those pages so they don't need any update. I'm just checking because they are our most used pages. E.g. https://foundation.mofostaging.net/en/campaigns/election-integrity-and-platform-accountability/ https://foundation.mofostaging.net/en/campaigns/tell-amazon-connected-devices-must-have-privacy-policies/

The first link is a Banner page with a Campaign Index page ancestor (parent), so root will be the page itself and its own title will show in H1.
The second link is a Campaign page which is not affected.

With regard to the last three comments related to staging, none of my changes are on staging and these seem to related to the menu rather than the H1, so maybe we discuss and spin off into a separate issue?
#7637 (comment)
#7637 (comment)
#7637 (comment)

Note
Just to clarify, my changes in this PR only remove the header variable from the template that outputs the H1. I haven't changed the template that outputs the menu component or removed the header field from any actual pages in the CMS.

Phew.

@Pomax @kristinashu
Would you both be happy to have a call based on the above? Let me know if I've missed anything or something needs further clarification.

[tbrlpld]

Approving this to check Percy integration