Implement fallback for empty links in HMRC Manuals #3090
Conversation
| @@ -27,7 +27,7 @@ | |||
| <% change_notes.each do |change_note_entry| %> | |||
| <% change_note = change_note_entry.flatten.last %> | |||
| <p class="govuk-body"> | |||
| <%= link_to change_note["title"], change_note["base_path"], class: "govuk-link" %> | |||
| <%= link_to change_note["title"].present? ? change_note["title"] : change_note["base_path"], change_note["base_path"], class: "govuk-link" %> | |||
There was a problem hiding this comment.
Hmmm. I don't think you want to display the base path as a link though? You'd still want human readable text in the link. Is there anything else available? Or perhaps you'd could use some generic text?
There was a problem hiding this comment.
I agree with Leena that this looks ugly and will sound horrible to screenreaders. But I appreciate we don't have much to work with. We can't use generic text because that would potentially lead to the same text multiple times on a page pointing to different urls, which would be a different accessibility fail. We could do something like extracting the last part of the url and linking "esm2055a" but that too is largely impenetrable. So another option is to just not put the link in if the title hasn't been supplied. To the sighted user the page will look the same as it does now, but we remove the bug of an empty link.
I recommend getting input from content people on which of the two options would be preferable - ugly url links, or no link at all.
There was a problem hiding this comment.
Yes, I agree with both comments TBH. I'm referring it back to our team to look at from a content point of view. The only counter argument might be that this is not that common an occurrence and we are considering a longer term fix as well along the lines of making title a required field so it's always present.
There was a problem hiding this comment.
I had a quick look into seeing if the base path could be made into a readable string. It got ugly fast:
irb(main):032> base_path = "/hmrc-internal-manuals/employment-status-manual/esm2055a"
=> "/hmrc-internal-manuals/employment-status-manual/esm2055a"
irb(main):033> base_path.split(/[\/]/).drop(2).join(" ").split("-").join(" ").humanize
=> "Employment status manual esm2055a"There was a problem hiding this comment.
Thanks @leenagupte - we're putting this on hold for the minute to further discuss the best way forward but this will be useful for that discussion. 👍
There was a problem hiding this comment.
@leenagupte @maxgds I've updated this PR that we were discussing last week. Since then we have decided to not render a link at all where that link would have blank text content. It's still not a great solution but deemed to be the least worst: it means the affected entries are visually the same as they currently are but removing the link from the markup prevents the accessibility failure.
davidtrussler
force-pushed
the
849_Implement-fallback-for-empty-links
branch
from
6b4b952 to
5e2dec7
Compare
davidtrussler
changed the title
base_path where title not present
There was a problem hiding this comment.
Could we add an extra test in here?
You would need to make a small change to this file which is the example being fetched for the test. eg:
{
"change_note": "Updated content",
"section_id": "VATGPB5390",
"title": "",
"published_at": "2015-03-05T15:16:20+00:00",
"base_path": "/hmrc-internal-manuals/vat-government-and-public-bodies/vatgpb5390"
},
@leenagupte The basepath is being added by HMRC Manuals API here |
Trello
This deals with an accessibility fail whereby a link to the amended document is not being displayed, see this page, under the entry for 5 October 2023.
This happens because there is no value entered in the data for
title, which is used as the text content for the link. There is a outstanding suggestion to force this to be a required value so that this does not occur but as a more immediate fix to deal with existing instances of the problem it was decided to not render the link at all in these cases. This retains the current visual appearance of the page (see below) but removes the cause of the accessibility fail (the blank link).Follow these steps if you are doing a Rails upgrade.