Bug 27740 - Accessibility: OPAC - Headings should have correct tags and hierarchy
Summary: Accessibility: OPAC - Headings should have correct tags and hierarchy
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Wainui Witika-Park
QA Contact: Testopia
URL:
Keywords: accessibility
Depends on: 27610
Blocks: 27876 27882 28242
  Show dependency treegraph
 
Reported: 2021-02-21 21:00 UTC by Wainui Witika-Park
Modified: 2024-07-25 11:18 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
21.05.00
Circulation function:


Attachments
Bug 27740: Corrected tags & hierarchy of headings (56.83 KB, patch)
2021-03-04 02:12 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: Corrected tags & hierarchy of headings (56.89 KB, patch)
2021-03-05 00:47 UTC, Hayley Pelham
Details | Diff | Splinter Review
Bug 27740: Corrected tags & hierarchy of headings (54.46 KB, patch)
2021-03-26 02:08 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: Corrected tags & hierarchy of headings (54.51 KB, patch)
2021-03-26 14:23 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27740: Corrected tags & hierarchy of headings (54.48 KB, patch)
2021-04-15 14:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27740: (QA follow-up) Fix 'Payment method' (1.47 KB, patch)
2021-04-15 14:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27740: (QA follow-up) Fix 'Your purchase suggestions' (2.63 KB, patch)
2021-04-15 14:46 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27740: Corrected tags & hierarchy of headings (54.57 KB, patch)
2021-04-16 02:19 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: (QA follow-up) Fix 'Payment method' (1.54 KB, patch)
2021-04-16 02:24 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: (QA follow-up) Fix 'Your purchase suggestions' (2.70 KB, patch)
2021-04-16 02:24 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: (follow-up) changed welcome user to <p> (1.76 KB, patch)
2021-04-16 03:16 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: (follow-up) quote of the day to <h2> (1.46 KB, patch)
2021-04-16 03:24 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: Added User summary heading (1.30 KB, patch)
2021-04-18 07:59 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: (follow-up) Added User summary heading (1.31 KB, patch)
2021-04-18 08:01 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: (follow-up) changed welcome user to <p> (1.82 KB, patch)
2021-04-21 14:47 UTC, Henry Bolshaw
Details | Diff | Splinter Review
Bug 27740: (follow-up) quote of the day to <h2> (1.51 KB, patch)
2021-04-21 14:47 UTC, Henry Bolshaw
Details | Diff | Splinter Review
Bug 27740: (follow-up) Added User summary heading (1.37 KB, patch)
2021-04-21 14:47 UTC, Henry Bolshaw
Details | Diff | Splinter Review
Bug 27740: (follow-up) changed "User summary" css (1.04 KB, patch)
2021-04-21 15:26 UTC, Henry Bolshaw
Details | Diff | Splinter Review
Bug 27740: (follow-up) changed "User summary" css (1.11 KB, patch)
2021-04-22 00:59 UTC, Wainui Witika-Park
Details | Diff | Splinter Review
Bug 27740: Corrected tags & hierarchy of headings (54.46 KB, patch)
2021-05-04 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27740: (QA follow-up) Fix 'Payment method' (1.60 KB, patch)
2021-05-04 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27740: (QA follow-up) Fix 'Your purchase suggestions' (2.76 KB, patch)
2021-05-04 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27740: (follow-up) changed welcome user to <p> (1.88 KB, patch)
2021-05-04 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27740: (follow-up) quote of the day to <h2> (1.58 KB, patch)
2021-05-04 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27740: (follow-up) Added User summary heading (1.43 KB, patch)
2021-05-04 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 27740: (follow-up) changed "User summary" css (1.18 KB, patch)
2021-05-04 10:58 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Wainui Witika-Park 2021-02-21 21:00:29 UTC
Each page of the OPAC should contain correct headings with correct heading tags and hierarchy. Please see https://www.w3.org/WAI/tutorials/page-structure/headings/ for more information.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27610 dealt with changing the logo on each page to be a <div> instead of an <h1> and then ensuring each page has one <h1> that tells users what the page is about.

This bug will deal with ensuring the rest of the headings in each page follow the correct heading hierarchy, and headings don't skip a level.
Comment 1 Wainui Witika-Park 2021-03-04 02:12:02 UTC
Created attachment 117629 [details] [review]
Bug 27740: Corrected tags & hierarchy of headings

Bug 27610 corrected the <h1> tags in each of the OPAC files. This patch
corrects the hierarchy of the rest of the headings on each of the
pages.

To test:
1) Go to the OPAC
2) Apply Bug 27610 patch
3) Apply patch (Bug 27740)
4) Go to each of the pages and check that the headings under <h1> are in
the correct order and hierarchy

Sponsored-by: Catalyst IT
Comment 2 Hayley Pelham 2021-03-05 00:47:49 UTC
Created attachment 117771 [details] [review]
Bug 27740: Corrected tags & hierarchy of headings

Bug 27610 corrected the <h1> tags in each of the OPAC files. This patch
corrects the hierarchy of the rest of the headings on each of the
pages.

To test:
1) Go to the OPAC
2) Apply Bug 27610 patch
3) Apply patch (Bug 27740)
4) Go to each of the pages and check that the headings under <h1> are in
the correct order and hierarchy

Sponsored-by: Catalyst IT
Signed-off-by: Hayley Pelham <hayleypelham@catalyst.net.nz>
Comment 3 Hayley Pelham 2021-03-05 00:51:01 UTC
Hello everyone,

I've signed off this patch to indicate that it's been checked through and tested and everything's looking good semantically.

As the goal for this bug is to correct the existing hierarchy of headings, we can deal with any redundancy (or locations where headings are used they perhaps shouldn't be) on a separate bug report.

As Wainui and I are both with Catalyst IT it'd be great if someone could take a look at this and sign it and its dependency Bug 27610 off or provide some feedback!
Comment 4 Hayley Pelham 2021-03-05 01:05:13 UTC
> As the goal for this bug is to correct the existing hierarchy of headings,
> we can deal with any redundancy (or locations where headings are used they
> perhaps shouldn't be) on a separate bug report.

Bug 27876 has been created for this purpose.
Comment 5 Jonathan Druart 2021-03-05 07:55:04 UTC
Owen, Martin, could you help this one (and bug 27610) to avoid unnecessary rebases?
Comment 6 Owen Leonard 2021-03-05 15:08:06 UTC
Some comments:

1. We don't need new CSS to hide the heading on the main page. We can add the Bootstrap class ".sr-only"

2. I don't think it's necessary to add headings inside <caption> and <legend>, e.g. opac-memberentry.tt:

- <legend id="mainaddress_legend">Main address</legend>
+ <legend id="mainaddress_legend"><h2>Main address</h2></legend>

Semantically speaking I think the legend is correct, and that the heading isn't necessary. Is there a particular accessibility tool behavior you're trying to address?

3. It is invalid to put a heading tag inside a <label> (e.g. opac-advsearch.tt). I have the same question as above about whether this change addresses some specific need.

4. I disagree that a heading shouldn't be used inside a message like this (opac-results.tt):

                          <div class="dialog alert">
-                            <h4>Error:</h4>
+                            <p><strong>Error:</strong></p>

4. This patch adds an invalid file: .opac-readingrecord.tt.swo
Comment 7 Jonathan Druart 2021-03-05 15:18:49 UTC
(In reply to Owen Leonard from comment #6)
> 4. This patch adds an invalid file: .opac-readingrecord.tt.swo

Check added to qa-test-tools
Issue #40 - Prevent .swo file to be added to the codebase
Comment 8 Wainui Witika-Park 2021-03-07 21:07:50 UTC
(In reply to Owen Leonard from comment #6)
> Some comments:
> 
> 1. We don't need new CSS to hide the heading on the main page. We can add
> the Bootstrap class ".sr-only"
> 
> 2. I don't think it's necessary to add headings inside <caption> and
> <legend>, e.g. opac-memberentry.tt:
> 
> - <legend id="mainaddress_legend">Main address</legend>
> + <legend id="mainaddress_legend"><h2>Main address</h2></legend>
> 
> Semantically speaking I think the legend is correct, and that the heading
> isn't necessary. Is there a particular accessibility tool behavior you're
> trying to address?
> 
> 3. It is invalid to put a heading tag inside a <label> (e.g.
> opac-advsearch.tt). I have the same question as above about whether this
> change addresses some specific need.
> 
> 4. I disagree that a heading shouldn't be used inside a message like this
> (opac-results.tt):
> 
>                           <div class="dialog alert">
> -                            <h4>Error:</h4>
> +                            <p><strong>Error:</strong></p>
> 
> 4. This patch adds an invalid file: .opac-readingrecord.tt.swo

Thanks so much for testing, will have a look into those things now.
Comment 9 Wainui Witika-Park 2021-03-07 22:45:19 UTC
(In reply to Owen Leonard from comment #6)
> Some comments:
> 
> 1. We don't need new CSS to hide the heading on the main page. We can add
> the Bootstrap class ".sr-only"
> 

Okay cool, didn't know about that class. Thanks, will amend that.

> 2. I don't think it's necessary to add headings inside <caption> and
> <legend>, e.g. opac-memberentry.tt:
> 
> - <legend id="mainaddress_legend">Main address</legend>
> + <legend id="mainaddress_legend"><h2>Main address</h2></legend>
> 
> Semantically speaking I think the legend is correct, and that the heading
> isn't necessary. Is there a particular accessibility tool behavior you're
> trying to address?
> 
> 3. It is invalid to put a heading tag inside a <label> (e.g.
> opac-advsearch.tt). I have the same question as above about whether this
> change addresses some specific need.

The reason we are putting heading tags in things such as legends and labels is that they appear to visually be heading a particular block on a page.

We out the <h> tags inside the legends and labels as we were not sure what the best way to include them was, and we didn't want to remove the legend and label tags.

 With screen readers, it is easiest for a person to scan through the page by headings, to get an idea about the certain areas of the page.

We don't believe it is possible to scan through the page using legends or labels, and even if it is possible, we think that this is an inconvenience to screen reader users as they would have to be aware of the legends and labels to know to do this.

If you are aware of a better way to include these headings, please let us know. One option we thought of was to possibly remove the headings from the labels and legends and put them outside and hide them.

> 
> 4. I disagree that a heading shouldn't be used inside a message like this
> (opac-results.tt):
> 
>                           <div class="dialog alert">
> -                            <h4>Error:</h4>
> +                            <p><strong>Error:</strong></p>

We will make a follow-up patch to move some of these alerts back to headings, but move them up levels so they are in the correct hierarchy.

Some of these however, are not meant to be headings as they have no information under them, so these do need to be dealt with.
> 
> 4. This patch adds an invalid file: .opac-readingrecord.tt.swo

Oh yes thanks, will remove this.

Thanks again for all your feedback!
Comment 10 Wainui Witika-Park 2021-03-24 21:53:31 UTC
Hi Owen,

I have contacted our Accessibility team at Catalyst and have received the following message:

-------------------------------------------------------------------------

"First, I would make sure you're using the proper HTML. According to MDN,
<h#> is a permitted child of <legend> and <caption>, but not of <label>.
Instead, the <h#> would be the parent of <label>.

Second, consider the context of the headings.

According to W3C, "Headings communicate the organization of the content
on the page" so yes I do think that headings can help screen reader
users to have an overview of the website
(https://www.w3.org/WAI/tutorials/page-structure/headings/). However,
headings should be used to identify sections of the page instead of
providing name or specify title of an element.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/label
"Placing heading elements within a <label> interferes with many kinds of
assistive technology, because headings are commonly used as a navigation
aid. If the label's text needs to be adjusted visually, use CSS classes
applied to the <label> element instead."

You can look at this link to see examples of "Making labels and legends
headings"
https://design-system.service.gov.uk/get-started/labels-legends-headings/"

-------------------------------------------------------------------------

Therefore, I am going to have another look at everything and keep headings where they are specific sections of the page, as this helps screen-readers navigate a webpage. However, if the element of the page doesn't seem to be a stand alone section, I will amend this to put it back to a legend, caption or label.
Comment 11 Wainui Witika-Park 2021-03-26 02:08:18 UTC
Created attachment 118837 [details] [review]
Bug 27740: Corrected tags & hierarchy of headings

Bug 27610 corrected the <h1> tags in each of the OPAC files. This patch
corrects the hierarchy of the rest of the headings on each of the
pages.

To test:
1) Go to the OPAC
2) Apply Bug 27610 patch
3) Apply patch (Bug 27740)
4) Go to each of the pages and check that the headings under <h1> are in the correct order and hierarchy

Sponsored-by: Catalyst IT
Comment 12 Owen Leonard 2021-03-26 14:23:13 UTC
Created attachment 118858 [details] [review]
Bug 27740: Corrected tags & hierarchy of headings

Bug 27610 corrected the <h1> tags in each of the OPAC files. This patch
corrects the hierarchy of the rest of the headings on each of the pages.

To test:
1) Go to the OPAC
2) Apply Bug 27610 patch
3) Apply patch (Bug 27740)
4) Go to each of the pages and check that the headings under <h1> are in
   the correct order and hierarchy

Sponsored-by: Catalyst IT

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 13 Martin Renvoize (ashimema) 2021-04-15 14:46:47 UTC
Created attachment 119633 [details] [review]
Bug 27740: Corrected tags & hierarchy of headings

Bug 27610 corrected the <h1> tags in each of the OPAC files. This patch
corrects the hierarchy of the rest of the headings on each of the pages.

To test:
1) Go to the OPAC
2) Apply Bug 27610 patch
3) Apply patch (Bug 27740)
4) Go to each of the pages and check that the headings under <h1> are in
   the correct order and hierarchy

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 14 Martin Renvoize (ashimema) 2021-04-15 14:46:50 UTC
Created attachment 119634 [details] [review]
Bug 27740: (QA follow-up) Fix 'Payment method'

The 'Payment method' heading now needs to be moved to the second level
to ensure consistent header heirarchy.

Test plan
1/ Add some debts to a patrons account
2/ Enable online payments (Paypal is a good example)
3/ Login to the OPAC and navigate to the 'Charges' page
4/ Note the header hierarchy has no gaps... (h1 -> h2, not h1 -> h3)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize (ashimema) 2021-04-15 14:46:54 UTC
Created attachment 119635 [details] [review]
Bug 27740: (QA follow-up) Fix 'Your purchase suggestions'

On the purchase suggestions page you could end up with two "Your
purchase suggestions" headings, one as an h1 and one as an h2..

Test plan
1/ Add at least one purchage suggestion to a patron
2/ Navigate to 'Purchase suggestions'
3/ Note that just one 'Your purchase suggestsions' heading appears with
a top level '1'.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize (ashimema) 2021-04-15 14:50:25 UTC
OK, So, I added a couple of follow-ups that would be worthwhile checking.

I also notice that on the opac-main page, once logged in you can be presented with a malformed header hierarchy still..

1. The 'user_summary' block, where is says "Welcome, user" in the top of the box on the right side of the page.. that's an 'h3'.. I'm not sure it should be a header element at all.. it's a 'box title'.. thoughts?
2. If the quote of the day feature is enabled, we display a 'Quote of the day' heading above all over content in the 'main' content area of the page.. that is also an 'h3' which doesn't feel right... maybe this should be converted to more of a clear 'content box, with a title' again, instead of looking like part of the main content of the page with a standard header?

Given the above I think I need some feedback before being able to PQA
Comment 17 Wainui Witika-Park 2021-04-16 02:19:38 UTC
Created attachment 119687 [details] [review]
Bug 27740: Corrected tags & hierarchy of headings

Bug 27610 corrected the <h1> tags in each of the OPAC files. This patch
corrects the hierarchy of the rest of the headings on each of the pages.

To test:
1) Go to the OPAC
2) Apply Bug 27610 patch
3) Apply patch (Bug 27740)
4) Go to each of the pages and check that the headings under <h1> are in
   the correct order and hierarchy

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Wainui Witika-Park 2021-04-16 02:24:13 UTC
Created attachment 119688 [details] [review]
Bug 27740: (QA follow-up) Fix 'Payment method'

The 'Payment method' heading now needs to be moved to the second level
to ensure consistent header heirarchy.

Test plan
1/ Add some debts to a patrons account
2/ Enable online payments (Paypal is a good example)
3/ Login to the OPAC and navigate to the 'Charges' page
4/ Note the header hierarchy has no gaps... (h1 -> h2, not h1 -> h3)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>
Comment 19 Wainui Witika-Park 2021-04-16 02:24:18 UTC
Created attachment 119689 [details] [review]
Bug 27740: (QA follow-up) Fix 'Your purchase suggestions'

On the purchase suggestions page you could end up with two "Your
purchase suggestions" headings, one as an h1 and one as an h2..

Test plan
1/ Add at least one purchage suggestion to a patron
2/ Navigate to 'Purchase suggestions'
3/ Note that just one 'Your purchase suggestsions' heading appears with
a top level '1'.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>
Comment 20 Wainui Witika-Park 2021-04-16 03:07:16 UTC
(In reply to Martin Renvoize from comment #16)
> OK, So, I added a couple of follow-ups that would be worthwhile checking.
> 
> I also notice that on the opac-main page, once logged in you can be
> presented with a malformed header hierarchy still..
> 
> 1. The 'user_summary' block, where is says "Welcome, user" in the top of the
> box on the right side of the page.. that's an 'h3'.. I'm not sure it should
> be a header element at all.. it's a 'box title'.. thoughts?
> 2. If the quote of the day feature is enabled, we display a 'Quote of the
> day' heading above all over content in the 'main' content area of the page..
> that is also an 'h3' which doesn't feel right... maybe this should be
> converted to more of a clear 'content box, with a title' again, instead of
> looking like part of the main content of the page with a standard header?
> 
> Given the above I think I need some feedback before being able to PQA

Hi Martin!

Thanks so much, I tested and signed off your 2 follow-up patches.

Yes I agree that the "Welcome, user" shouldn't actually even be a heading. I can make a follow up patch to amend this now.

I also agree the Quote of the day is incorrect so I'll make a follow up patch to amend that too.

Thanks again!
Comment 21 Wainui Witika-Park 2021-04-16 03:16:22 UTC
Created attachment 119690 [details] [review]
Bug 27740: (follow-up) changed welcome user to <p>

In the 'user_summary block', the "Welcome, user" was an <h3>. This has
been changed to a <p> as it isn't really a heading.

The styling can be changed to make it appear as it was before, or to
make it appear more significant on the page.

To test:
1) Go to the OPAC
2) Go to the home page
3) Go to the 'user_summary' block
4) Check that the part where is says "Welcome, user" is now a <p>

Sponsored-by: Catalyst IT
Comment 22 Wainui Witika-Park 2021-04-16 03:24:38 UTC
Created attachment 119691 [details] [review]
Bug 27740: (follow-up) quote of the day to <h2>

If the quote of the day feature is enabled, a "Quote of the day" heading
is displayed. It was an <h3> which was not proper hierarchy, so this has
been changed to an <h2>.

To test:
1) Go to staff client
2) Enable quote of the day preference
3) Go to the OPAC
4) Observe "Quote of the day" heading
5) Ensure it is an <h2>

Sponsored-by: Catalyst IT
Comment 23 Wainui Witika-Park 2021-04-16 03:25:37 UTC
Hi Martin, please I added 2 follow-up patches to fix the above concerns. However, please feel free to change them if you had other/better ideas in mind :)
Comment 24 Henry Bolshaw 2021-04-16 09:38:49 UTC
This is really great work, Wainui!

On the "user_summary" block, I agree that "Welcome, user" isn't really appropriate text for a heading but I think we do need a heading in that area to help users navigate to that section of the page. The user_summary block comes at the bottom of the page structure and I think it would be much harder to navigate to without a heading and the fact that we style it to be quite visually prominent suggests it's an important part of the page.

If we remove the heading from "Welcome, User" I'd prefer an additional heading sitting above it that says something like "My Account Summary" or "My Loans". That way you can use the headings option on a screen reader to quickly navigate to that part of the page.

But I don't want to stop momentum on this bug if others disagree.
Comment 25 Wainui Witika-Park 2021-04-18 07:59:51 UTC
Created attachment 119815 [details] [review]
Bug 27740: Added User summary heading

Added a User summary <h2>

To test:
1) Go to OPAC
2) Go to home page
3) Check that there is a <h2> saying "User summary"

Sponsored-by: Catalyst IT
Comment 26 Wainui Witika-Park 2021-04-18 08:01:54 UTC
Created attachment 119816 [details] [review]
Bug 27740: (follow-up) Added User summary heading

Added a User summary <h2>

To test:
1) Go to OPAC
2) Go to home page
3) Check that there is a <h2> saying "User summary"

Sponsored-by: Catalyst IT
Comment 27 Wainui Witika-Park 2021-04-18 08:04:27 UTC
(In reply to Henry Bolshaw from comment #24)
> This is really great work, Wainui!
> 
> On the "user_summary" block, I agree that "Welcome, user" isn't really
> appropriate text for a heading but I think we do need a heading in that area
> to help users navigate to that section of the page. The user_summary block
> comes at the bottom of the page structure and I think it would be much
> harder to navigate to without a heading and the fact that we style it to be
> quite visually prominent suggests it's an important part of the page.
> 
> If we remove the heading from "Welcome, User" I'd prefer an additional
> heading sitting above it that says something like "My Account Summary" or
> "My Loans". That way you can use the headings option on a screen reader to
> quickly navigate to that part of the page.
> 
> But I don't want to stop momentum on this bug if others disagree.

Hi Henry, thanks so much for your input!

I totally agree with you so have added an additional follow-up patch. Let me know what you think :)

Thanks!
Comment 28 Henry Bolshaw 2021-04-21 14:47:26 UTC
Created attachment 119953 [details] [review]
Bug 27740: (follow-up) changed welcome user to <p>

In the 'user_summary block', the "Welcome, user" was an <h3>. This has
been changed to a <p> as it isn't really a heading.

The styling can be changed to make it appear as it was before, or to
make it appear more significant on the page.

To test:
1) Go to the OPAC
2) Go to the home page
3) Go to the 'user_summary' block
4) Check that the part where is says "Welcome, user" is now a <p>

Sponsored-by: Catalyst IT

Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk>
Comment 29 Henry Bolshaw 2021-04-21 14:47:30 UTC
Created attachment 119954 [details] [review]
Bug 27740: (follow-up) quote of the day to <h2>

If the quote of the day feature is enabled, a "Quote of the day" heading
is displayed. It was an <h3> which was not proper hierarchy, so this has
been changed to an <h2>.

To test:
1) Go to staff client
2) Enable quote of the day preference
3) Go to the OPAC
4) Observe "Quote of the day" heading
5) Ensure it is an <h2>

Sponsored-by: Catalyst IT

Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk>
Comment 30 Henry Bolshaw 2021-04-21 14:47:35 UTC
Created attachment 119955 [details] [review]
Bug 27740: (follow-up) Added User summary heading

Added a User summary <h2>

To test:
1) Go to OPAC
2) Go to home page
3) Check that there is a <h2> saying "User summary"

Sponsored-by: Catalyst IT

Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk>
Comment 31 Jonathan Druart 2021-04-21 15:03:08 UTC
It would be great to have this one for 21.05, keep going :)
Comment 32 Henry Bolshaw 2021-04-21 15:26:47 UTC
Created attachment 119958 [details] [review]
Bug 27740: (follow-up) changed "User summary" css

Changed user summary CSS to style H2 not H3 heading

To test:
1) Go to OPAC homepage
2) Login as a user with at least one loan
3) See that the "User summary" heading is not styled
4) Apply patch
5) Run yarn build --view OPAC
6) Confirm the heading is now styled with grey background
Comment 33 Henry Bolshaw 2021-04-21 15:47:29 UTC
Wainui, I've signed off your patches and added a follow up to tweak the opac.scss so that the new H2 heading is styled the same way as the old 'Welcome, user' heading.

The 'welcome, user' text still looks slightly adrift but I'm not sure how to make it look better.

The current heading styling on the OPAC has an insufficient contrast ratio for accessibility purposes. I've created bug 28188 to collect all of these instances.
Comment 34 Wainui Witika-Park 2021-04-22 00:59:18 UTC
Created attachment 119981 [details] [review]
Bug 27740: (follow-up) changed "User summary" css

Changed user summary CSS to style H2 not H3 heading

To test:
1) Go to OPAC homepage
2) Login as a user with at least one loan
3) See that the "User summary" heading is not styled
4) Apply patch
5) Run yarn build --view OPAC
6) Confirm the heading is now styled with grey background

Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>
Comment 35 Martin Renvoize (ashimema) 2021-05-04 10:58:18 UTC
Created attachment 120437 [details] [review]
Bug 27740: Corrected tags & hierarchy of headings

Bug 27610 corrected the <h1> tags in each of the OPAC files. This patch
corrects the hierarchy of the rest of the headings on each of the pages.

To test:
1) Go to the OPAC
2) Apply Bug 27610 patch
3) Apply patch (Bug 27740)
4) Go to each of the pages and check that the headings under <h1> are in
   the correct order and hierarchy

Sponsored-by: Catalyst IT
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 36 Martin Renvoize (ashimema) 2021-05-04 10:58:22 UTC
Created attachment 120438 [details] [review]
Bug 27740: (QA follow-up) Fix 'Payment method'

The 'Payment method' heading now needs to be moved to the second level
to ensure consistent header heirarchy.

Test plan
1/ Add some debts to a patrons account
2/ Enable online payments (Paypal is a good example)
3/ Login to the OPAC and navigate to the 'Charges' page
4/ Note the header hierarchy has no gaps... (h1 -> h2, not h1 -> h3)

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 37 Martin Renvoize (ashimema) 2021-05-04 10:58:27 UTC
Created attachment 120439 [details] [review]
Bug 27740: (QA follow-up) Fix 'Your purchase suggestions'

On the purchase suggestions page you could end up with two "Your
purchase suggestions" headings, one as an h1 and one as an h2..

Test plan
1/ Add at least one purchage suggestion to a patron
2/ Navigate to 'Purchase suggestions'
3/ Note that just one 'Your purchase suggestsions' heading appears with
a top level '1'.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 38 Martin Renvoize (ashimema) 2021-05-04 10:58:31 UTC
Created attachment 120440 [details] [review]
Bug 27740: (follow-up) changed welcome user to <p>

In the 'user_summary block', the "Welcome, user" was an <h3>. This has
been changed to a <p> as it isn't really a heading.

The styling can be changed to make it appear as it was before, or to
make it appear more significant on the page.

To test:
1) Go to the OPAC
2) Go to the home page
3) Go to the 'user_summary' block
4) Check that the part where is says "Welcome, user" is now a <p>

Sponsored-by: Catalyst IT
Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 39 Martin Renvoize (ashimema) 2021-05-04 10:58:35 UTC
Created attachment 120441 [details] [review]
Bug 27740: (follow-up) quote of the day to <h2>

If the quote of the day feature is enabled, a "Quote of the day" heading
is displayed. It was an <h3> which was not proper hierarchy, so this has
been changed to an <h2>.

To test:
1) Go to staff client
2) Enable quote of the day preference
3) Go to the OPAC
4) Observe "Quote of the day" heading
5) Ensure it is an <h2>

Sponsored-by: Catalyst IT
Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 40 Martin Renvoize (ashimema) 2021-05-04 10:58:39 UTC
Created attachment 120442 [details] [review]
Bug 27740: (follow-up) Added User summary heading

Added a User summary <h2>

To test:
1) Go to OPAC
2) Go to home page
3) Check that there is a <h2> saying "User summary"

Sponsored-by: Catalyst IT
Signed-off-by: Henry Bolshaw <bolshawh@parliament.uk>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 41 Martin Renvoize (ashimema) 2021-05-04 10:58:43 UTC
Created attachment 120443 [details] [review]
Bug 27740: (follow-up) changed "User summary" css

Changed user summary CSS to style H2 not H3 heading

To test:
1) Go to OPAC homepage
2) Login as a user with at least one loan
3) See that the "User summary" heading is not styled
4) Apply patch
5) Run yarn build --view OPAC
6) Confirm the heading is now styled with grey background

Signed-off-by: Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 42 Martin Renvoize (ashimema) 2021-05-04 11:01:10 UTC
Great work team!

I can't spot any further issues, but with Koha as "optional" as it is with various preferences and options changing what's displayed on the page I wouldn't be at all surprised if between us we've missed an occurrence somewhere with a specific setup.

However, I think this is a great foundation and should be pushed as is.. we should just keep an eye out for bugs going forward.

So.. with that said.. PQA!
Comment 43 Jonathan Druart 2021-05-04 13:24:39 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 44 Fridolin Somers 2021-05-11 14:00:43 UTC
Enhancement not pushed to 20.11.x