WSTEAMA-1244 P2 video promos have no heading semantics #11735
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…, simple curation
… of github.com:bbc/simorgh into WSTEAMA-1244-P2-video-promos-have-no-heading-semantics
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves JIRA [1244]
Overall changes
To fix the bug where an h tag was empty (e.g 'h' instead of 'h2' or 'h3') change the code that passes though a variable that can be a blank string instead of a number.
In the previous code 'headingLevel={curationTitle && 3}' was being passed into the curation. The default argument for headingLevel was 2, but if curationTitle is an empty string (in some rare cases), curationTitle is falsy but has content, and the empty string is passed into the Curation instead of '3' or the default being used of '2'. I have also made this change to the HomePage file as it does the same thing and so could have the same bug.
To stop it being possible for an empty string to be passed in I have changed the code to
headingLevel={curationTitle && curationTitle.length ? 3 : 2} which means that if the curationTitle is an empty string, the number 2 is passed through to Curation instead.
Then looking through the code I also wasn't sure why we were using a variable variable (!) instead of a hard coded number when I couldn't think of a case when it would need to differ.
If there are multiple curations, we always want the promos to have heading level h3s, as the curations would be given titles which are h2s. Curations do not show if they do not have a title, unless they are the first curation. The first curation has a visually hidden title which is an h2.
In the case of a single curation, the title does not render (visually or hidden), so all the promos need to be h2s.
I hard coded these numbers in instead.
Curations with no summaries also go to the 'return null' case now instead of falling into a case rendering a grid component.
Is the Home Page logic supposed to be any different from this?
I have added tests for these heading scenarios for simple and hierarchical curations on the topic page.
Code changes
Testing
Check headings all have a number on topic and home pages.
Check curation subheadings are h2, and the promos underneath are h3.
Check promos have h2 headings when there is only 1 curation.
Do this for topic and home pages.
Helpful Links
Add Links to useful resources related to this PR if applicable.
Coding Standards
Repository use guidelines