(Angular) Staff Catalog Needs Option to Disable Highlighting

Bug #1970946 reported by John Amundson
52
This bug affects 11 people
Affects Status Importance Assigned to Milestone
Evergreen
Fix Released
Medium
Unassigned
3.11
Fix Released
Undecided
Unassigned

Bug Description

3.7.2ish

There is currently no checkbox to disable highlighting in the angular staff catalog. There should be a way to disable the feature similar to the public catalog.

Related bugs:
https://bugs.launchpad.net/evergreen/+bug/1783408
https://bugs.launchpad.net/evergreen/+bug/1910440

Revision history for this message
Benjamin Kalish (bkalish) wrote :

I absolutely support this feature, but I would also add that if the highlight color provided higher contrast with the text less folks would feel the need to turn off highlighting. It's far more important that folks be able to read the word than it is that they are able to distinguish the highlights background color from its surroundings.

In addition to the color change, we might consider using the mark element for highlighting (https://developer.mozilla.org/en-US/docs/Web/HTML/Element/mark)

Revision history for this message
John Amundson (jamundson) wrote :

Related bug on link color and accessibility on Angular pages - https://bugs.launchpad.net/evergreen/+bug/1960526

Changed in evergreen:
status: New → Confirmed
tags: added: accessibility usability
Revision history for this message
Stephanie Leary (stephanieleary) wrote :

Digging into this a little--the <mark> tag would be more semantic than <b> (the PostgreSQL default for match highlights) but neither is announced to screen readers:
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/mark#accessibility_concerns
https://accessibility.psu.edu/boldfacehtml/

Google uses <em>, which isn't announced either.

Does anyone have user feedback on the desired screen reader experience with match highlights? If there is a prefix we can add that would be helpful, we can--though I would adjust TGPi's generated CSS to use a data attribute on the HTML tag, so we can more easily localize it.
https://www.tpgi.com/short-note-on-making-your-mark-more-accessible/

As for the contrast, what colors we choose for the highlights depends on what we do with link colors, as per bug 1991562.

Revision history for this message
Stephanie Leary (stephanieleary) wrote :

I've updated my Bootstrap 5 color contrast demo to include search result highlighting:
https://codepen.io/stephanieleary/full/poVaPYq

Revision history for this message
Stephanie Leary (stephanieleary) wrote :

Noting that TPGi have updated their documentation on this, and JAWS and NVDA do announce the start and end of the highlight when <mark> is used:
https://www.tpgi.com/screen-readers-support-for-text-level-html-semantics/

Revision history for this message
Stephanie Leary (stephanieleary) wrote :

This branch includes both lighter colors in the CSS and a change to the SQL to use <mark> instead of <b> for match highlights.
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sleary/lp1970946-search-result-highlight-contrast

tags: added: pullrequest
Revision history for this message
John Amundson (jamundson) wrote :

Testing this on https://butternut.evergreencatalog.com/ for Bug Squashing Week. The highlight color looks a bit different, but I'm not sure if the initial concern was addressed. I don't see any way to disable highlighting in the staff catalog. Am I missing something?

Revision history for this message
Terran McCanna (tmccanna) wrote :

I agree that although the change is good, it doesn't address the original report. Removing pullrequest - perhaps Stephanie's change can be addressed in a new ticket?

tags: removed: pullrequest
Revision history for this message
Stephanie Leary (stephanieleary) wrote :

Since better contrast had been discussed as an alternative to providing a user option, I went that route (alongside all the other color contrast patches I've been working on). Happy to move this to a different bug specifically for the contrast if we want to leave this one open for a user option to be addressed later.

Revision history for this message
Stephanie Leary (stephanieleary) wrote :

Separate bug for <mark> and highlight contrast: https://bugs.launchpad.net/evergreen/+bug/2009073

Revision history for this message
Terran McCanna (tmccanna) wrote :

Noting that in 3.12 staff catalog we are still missing an option to disable search term highlighting.

Marking this regression since it was available prior to the Angular Staff Catalog.

tags: added: regression
Changed in evergreen:
importance: Undecided → Medium
Steven Mayo (stmayo)
Changed in evergreen:
assignee: nobody → Steven Mayo (stmayo)
Revision history for this message
Steven Mayo (stmayo) wrote (last edit ):

After noticing how closely related a few of these search highlight bugs are I figured it made sense to fix them all at once. The commit specifically for this bug is the 2nd commit on this branch:
https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/smayo/lp1970946-Staff_Catalog_Disable_Highlight_Setting
Other bugs addressed by this branch are:
https://bugs.launchpad.net/evergreen/+bug/1783408
https://bugs.launchpad.net/evergreen/+bug/1910440

I added a new user setting to the database and a new checkbox in Search Preferences to control it. It's a user setting so that it's consistent for staff members who use both the staff client and OPAC.

Steps to test (this ticket):
[1] On the staff client, go to Cataloging -> Search the Catalog
[2] Search for any item
[3] Look at how much green there is
[4] Click on any bib in the results
[5] Look at how much green there is in the record summary
[6] Go Back to Results -> Search Preferences
[7] Observe the new option 'Highlight Search Terms' that is checked
[8] Uncheck 'Highlight Search Terms'
[9] Make a new search or go back to results
[10] Look at how much green there isn't
[11] Click on any bib in the results
[12] Look at how much green there isn't in the record summary
[13] Log out and back in to the same account again
[14] Make a new search and notice that the highlighting is just like you left it
[15] Log out and back in to a different user.
[16] Observe that the highlighting is on for the new account regardless of how you left the last one.

Changed in evergreen:
assignee: Steven Mayo (stmayo) → nobody
tags: added: pullrequest
Revision history for this message
Stephanie Leary (stephanieleary) wrote :

Steven, I played around with this yesterday, and it mostly does the thing! However, the logic around false and null isn't quite there yet. When I toggle the pref, the highlighting works as expected in the staff search results, but then if I return to the prefs screen, the checkbox is always selected no matter what. The OPAC isn't mirroring what's been chosen in the staff preferences; it seems to behave independently.

Can you give this another look?

The form labels for the two checkbox preferences have gotten a little jumbled somehow. I've added a tiny commit to clean those up, if you'd like to squash this into your work: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sleary/lp1970946-Staff_Catalog_Disable_Highlight_Setting-label-fixup

tags: added: needswork
Revision history for this message
Steven Mayo (stmayo) wrote :

Stephanie, I'm not having the problems you mention with the checkbox always being highlighted when you load into the page and the OPAC not mirroring the staff setting.
Those are, however, exactly what happen if you haven't run the SQL upgrade script in the second commit that adds the new user setting to the database.

I think you may not have run the SQL upgrade script and the error handling did a bad job telling you things were going wrong. The checkbox was altering your search temporarily even if it failed to update the database setting. I've squashed in a change to end that, along with your form label cleanup.
Also, the search preferences page hasn't been showing a failure toast when a setting update fails - I've squashed in a fix for that too. The toast should've been there if you did forget to run the SQL upgrade.

Branch: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/smayo/lp1970946-Staff_Catalog_Disable_Highlight_Setting

If I'm wrong, and you did run the SQL upgrade, whatever's happening is very weird.

Side note: Do we have/should we have a standard way of saying what restart steps will be necessary for a branch compared to main? Figuring out what steps are needed by checking every changed file in all the commits on a branch seems like extra mental load on testers.

tags: removed: needswork
Revision history for this message
Steven Mayo (stmayo) wrote :
Revision history for this message
Terran McCanna (tmccanna) wrote :

(Noting that this is the top TWO commits)

Revision history for this message
Stephanie Leary (stephanieleary) wrote :

Thanks, Steven! I evidently had run the upgrade script, since running it again gave me an error message, so I'm not sure what was going on with the first branch. However, this one seems to work perfectly!

Signoff branch, with a small edit to the commit message: https://git.evergreen-ils.org/?p=working/Evergreen.git;a=shortlog;h=refs/heads/user/sleary/lp1970946-signoff

tags: added: signedoff
Revision history for this message
Terran McCanna (tmccanna) wrote :

(Noting that this incorporates fixes for bug 1910440 and bug 1783408)

Changed in evergreen:
assignee: nobody → Terran McCanna (tmccanna)
Revision history for this message
Terran McCanna (tmccanna) wrote :

Merged this fix to main. I was hesitant to push it back as it does change the behavior somewhat.

Changed in evergreen:
milestone: none → 3.next
status: Confirmed → Fix Committed
assignee: Terran McCanna (tmccanna) → nobody
Changed in evergreen:
status: Fix Committed → Fix Released
status: Fix Released → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
Changed in evergreen:
status: Fix Released → Fix Committed
milestone: 3.next → 3.14-beta
Revision history for this message
Jason Stephenson (jstephenson) wrote :

This one ended up getting tricky. There are two branches and two fixes for 3 different bugs here.

The first branch mainly fixes this bug and was committed for release in 3.11.0 as near as I can tell.

The second branch fixes the two related bugs was not committed until June 2024.

I updated the targets to reflect this status as well as I can given the limitations of Launchpad:

The 3.11 series is set to Fix Released to reflect the time of the original patch.

The main series reflects the status of the additional branch to resolve the other two? bugs.

Revision history for this message
Andrea Neiman (aneiman) wrote :

Came across this while crunching numbers for EG Annual Report - looks like this was committed to rel_3_14 on 2024-06-10 and subsequently released with 3.14-beta so I'm marking this as fix released.

Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.