-
Notifications
You must be signed in to change notification settings - Fork 29
implement football more button for when js is disabled #13903
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.
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
base: main
Are you sure you want to change the base?
Conversation
Size Change: +50 B (+0.01%) Total Size: 973 kB ℹ️ View Unchanged
|
<div css={footballMatchesGridStyles}> | ||
<div | ||
<a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is minor, raising out of interest. Nesting a button inside a link is not valid HTML:
Content model:
Transparent, but there must be no interactive content descendant, a element descendant, or descendant with the tabindex attribute specified.
There might be weird behaviour as a result.
An alternative solution might be to retain the outer <div>
and only hide the <a>
if JS loads. I haven't tested this.
<div>
{
jsLoaded ?
(<Button></Button>)
:
(<LinkButton></LinkButton>)
}
</div>
What does this change?
This change adds support for a fallback
More
button on the football matches pages for when JavaScript is disabled in the browser.Why introduce nextPageNoJsUrl:
There is a convention in frontend routing where for any page route (e.g. /football/fixtures/more/:year/:month/:day), there is also a corresponding .json route (e.g. /football/fixtures/more/:year/:month/:day.json). These .json routes return the full data required to render a page, including config and navigation.
In DCAR the
More
button is using the .json route to fetch more matches. However, this returns far more data than necessary — config, nav, and other unrelated information — when all that is needed is a list of matches to append to the current view.Upon further investigation, we noticed that the HTML version of the
/more/
routes serves the same content as the equivalent non-/more/ routes. Based on this, we decided to remove the HTML versions of the /more/ routes to simplify routing. The PR for this: guardian/frontend#27939However, we later realised that the DCAR implementation did not support a non-JavaScript experience for the More button. To fix this, we needed a way to render the More button as a fallback link (i.e. an anchor tag) that simply navigates to the next page of matches when JavaScript is disabled.
Since the
/more/
routes are being deprecated, we now use the equivalent non-/more/ route to construct that fallback link — this is provided via the newnextPageNoJsUrl
prop.TOTO:
This PR can be merged after guardian/frontend#27946