SUL23-605: fine tuning the sul people page. #204

jenbreese · 2024-08-15T22:51:52Z

READY FOR REVIEW

Summary

The toggle functionality redone. "Toggle should only allow for one selection (like radio button group behavior)"
Made the search box full width on mobile.
Updated the font sizes for the toggles. SM is 14px. Medium and up is 16px
Updated the padding on the toggles.
Put a nowrap on the toggles

Review By (Date)

8/19

Criticality

Normal

Urgency

Normal

Review Tasks

Setup tasks and/or behavior to test

Navigate to https://su-library-git-sul23-605-specialists-stanford-libraries.vercel.app/test-list-page
Verify the changes above.

Site Configuration Sync

Is there a config:export in this PR that changes the config sync directory?

Front End Validation

Design is approved by @ user?
HTML validation: Is the markup using the appropriate semantic tags and passes validation? Or, QA request ticket created?
Cross-browser testing: Has been performed? Or, QA request ticket created?
Automated accessibility: Scans performed? Or, QA request ticket created?
Manual accessibility: Manually tested? Or, QA request ticket created?

Backend / Functional Validation

Code

Are the naming conventions following our standards?
Does the code have sufficient inline comments?
Is there anything in this code that would be hidden or hard to discover through the UI?
Are there any code smells?
Are tests provided? eg (unit, behat, or codeception)

Code security

Are all forms properly sanitized?
Any obvious security flaws or new areas for attack?

General

Is there anything included in this PR that is not related to the problem it is trying to solve?
Is the approach to the problem appropriate?

Affected Projects or Products

Does this PR impact any particular projects, products, or modules?

Associated Issues and/or People

- SUL23-601
- SUL23-605

Resources

AMP Tool
Accessibility Manual Test Script
HTML Validator
Browserstack and link to Browserstack Credentials

vercel · 2024-08-15T22:51:56Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
su-library	✅ Ready (Inspect)	Visit Preview	Aug 28, 2024 5:58pm

jenbreese · 2024-08-16T16:59:09Z

src/components/views/sul-people/sul-people-table-view.tsx

@@ -53,8 +53,10 @@ const SulPeopleTableView = ({items, hasHeading}: Props) => {
      const existingIndex = newState.indexOf(type)
      if (existingIndex >= 0) {
        newState.splice(existingIndex, 1)
+        console.log("newState splice line")


Hi @pookmish I want to change the toggle behavior for the specialist. The ticket says "Toggle should only allow for one selection (like radio button group behavior)". I changed the buttons from checkboxes to radio and removed the group hover because it seemed to confuse thing. I added console logs and it doesn't seem to go to the splice part of the if. and the link 59 logs out two comments per click.

Can you point me in the correct direction?

You are going to have to refactor a lot of the onClick and state setup to get such a thing to work.

Keep in mind that if we change them to radio buttons, we will have to show the "All Specialists" on mobile as well.

Yes, they want to have the All there now too. They changed it to just All.

Ok. Would you like me to update this PR using radios?

I was reading an article and trying to get it to work. Let me see if I figure it out.

Ok, can you update the PR with radios?

I've updated the PR. See the last commit to familiarize yourself with this approach. It's more simple than the checkboxes.
Note that you will need to have some "checked" states. Use peer-checked:.... on the <span> tags immediately after the <input> tags.

That was helpful to see. It is a lot simpler.

pookmish

I'm not sure if you intended to assign to me already. It still requires active states for each of the 3 options. I'll remove myself as assigned until you are ready.

jenbreese · 2024-08-17T00:04:53Z

That was a mistake. I added the background color/opacity on the hover and active state. Only the selected or focus toggle radio button needs to have the additional border. (I double checked the designs)

jenbreese · 2024-08-21T21:17:38Z

When you are back, this is ready for your review.

pookmish · 2024-08-22T14:41:10Z

src/components/views/sul-people/sul-people-table-view.tsx

              />
-              <span className="block items-center rounded-l-full border border-r-0 border-black-80 p-9 pr-20 text-18 no-underline group-hover:border-cardinal-red-dark group-hover:text-cardinal-red-dark group-hover:underline peer-checked:bg-[#979694] peer-checked:bg-opacity-20 peer-focus:border-2 peer-focus:border-black-80 peer-focus:bg-[#979694] peer-focus:bg-opacity-10 md:rounded-l-none">
+              <span className="block items-center whitespace-nowrap border border-r-0 border-black-80 p-4 px-16 text-14 leading-[30px] no-underline group-hover:border-cardinal-red-dark group-hover:text-cardinal-red-dark group-hover:underline peer-checked:bg-[#979694] peer-checked:bg-opacity-20 peer-hover:bg-[#979694] peer-hover:bg-opacity-10 peer-focus:border-2 peer-focus:border-black-80 peer-focus:bg-[#979694] peer-focus:bg-opacity-10 peer-active:bg-[#979694] peer-active:bg-opacity-10 md:rounded-l-none md:text-16">


As I mentioned, you need peer-checked. peer-active is only when the user is actively engaged on the element. as soon as they click/focus off the element, those styles are lost. Also, to my knowledge, a simple background color indicator as this is not sufficient for accessibility. The gray does not have a high enough contrast with the white to indicate its selected state.

Thank you. I see what you mean. They are going to make some changes to these toggles so I'll bring up the accessibility part and make those changes in this PR, unless you have concerns about that.

No concerns other than what I mentioned. If it helps anything, this is what we did on Press: https://stanford-university-press.vercel.app/books/award-winners

Also FWIW, the contrast needs to be 3:1

* SUL23-540: Updated robots.txt to disallow /all/ (#166) * Better automated tests & Gitpod by building a cloned site (#165) * Remove node cache to improve cache invalidation for config pages (#168) * SUL23-515 SUL23-541: Colors, fonts, word fixes to the toggles. (#169) * SUL23-515 SUL23-541: Colors, fonts, word fixes to the toggles. * Keyword search filter stubbed out (#156) * Keyword search filter stubbed out * SUL23-503: fixup to search box styles * fixup to styles * moved over the search cursor * Fix label * fixup to the button toggles --------- Co-authored-by: Jen Breese-Kauth <[email protected]> * SUL23-525: Updates to the display for Med breakpoint on Branches and Centers (#170) * SUL23-525: stashing work. * SUL23-525: using grid to get alignment and display for md view * Changed three tabular layouts to use the grid at the md breakpoint * fixup to comment * SUL23-438: Moved the location link and changed to Open or Closed until (#163) Co-authored-by: Mike Decker <[email protected]> * SUL23-479: added space (#176) * SUL23-549: places to study alignment (#172) * SUL23-549: places to study alignment * SUL23-570: adding the corrected times to the places to study table (#175) * SUL23-550: Move the condition so the td element remained even when empty (#171) * SUL23-550: Move the condition so the td elmement remained even when empty * fixup * SUL23-548 SUL23-553 SUL23-554 SUL23-555 SUL23-556 SUL23-547 (#173) * SUL23-548 SUL23-553 SUL23-554 SUL23-555 SUL23-556 SUL23-547 * fixup * fixup * fixup * SUL23-543: rounded corner * SUL23-557: adding aria-hidden and tabindex for a11y to photos on tabular data tables (#177) * SUL23-557: stashing work to ask about tomorrow * SUL23-557: fixup for the tabindex * fixup to branch page * SUL23-557: removed the link on the image in the study places * Adjust how library hours data is cached to reduce cache writes/reads (#178) * SUL23-503: search label font size (#180) * SUL23-570: changed address to size 16 (#179) * SUL23-580: nav fixup (#184) * SUL23-556: normal font and leading (#186) * SUL23-503 Add text reset button and no results display (#185) * SUL23-503 Add text reset button and no results display * SUL23-503: changed size, added padding and color * fixup after merge --------- Co-authored-by: Jen Breese-Kauth <[email protected]> Co-authored-by: Ian Monroe <[email protected]> * SUL23-577 SUL23-586: buttons and vertical centering on tables (#183) * SUL23-577 SUL23-586: buttons and vertical centering on tables * fixed the col spacing --------- Co-authored-by: Ian Monroe <[email protected]> * SUL23-576 SUL23-575 SUL23-574: fixed height of toggle and filters (#182) * SUL23-576 SUL23-575 SUL23-574: fixed height of toggle and filters * fixup to corner * SUL23-576: fixup * removed rounded idea * SUL23-591: added the th row=scope for a11y (#187) Co-authored-by: Ian Monroe <[email protected]> * Fix date formatting when the input is UTC * SUL23-551: sentence case for branches and places to study (#188) * SUL23-551: sentence case for branches and places to study * Sentence case for all locations * SUL23-482: set Todays hours title to h2 (#189) * SUL23-482: set Todays hours title to h2 * "Hours" needed to be sentence case * SUL23-587 SUL23-589 SUL23-590 (#190) * SUL23-587 SUL23-589 SUL23-590 * adding the correct padding after cell * SUL23-588: make the hover hug the phone and email * Moved to just lg for inline flex * fixup * SUL23-578: fixup to the margin-bottom and casing (#191) * SUL23-578: fixup to the margin-bottom and casing * adding the flex change from the parent branch * Fix date formatting when the input is UTC * SUL23-578: added the 2 digits to the closed until time and lowercase of until * fixup to the AM PM --------- Co-authored-by: Mike Decker <[email protected]> --------- Co-authored-by: Mike Decker <[email protected]> * SUL23-577: fixed button height. removed include not used (#192) * SUL23-587: updates to the people table (#193) * SUL23-587: updates to the people table * SUL23-440 Add "Load More" button on news lists over 30 items * Added missing fields for people teasers * SUL23-440 Adjusted "Load More" to "Load more" * SUL23-597 Fix "today" vs "tomorrow" vs "friday" next opening day * SUL23-597 Fix hour displays for next open date/time (#194) * Removed "Hours this week" text * SUL23-598: fixup up hover on branch name (#195) * SUL23-598: fixup up hover on branch name * fixup * SUL23-574: setting exact height (#196) * SUL23-580 SUL23-538: Added in the at to the time string (#197) * Handle empty response from api/library-hours if something fails * SUL23-585: Adding Donate now button to header (#198) * SUL23-585: Adding Donate now button to header * SUL23-585: button in header * fixup * changed Links to links * SUL23-585: Back to digital red (#201) * SUL23-571 Sort library options by title * SUL23-600 show closing minutes if they are not 00 (#202) * SUL-23-570: adding Hours this week for default (#203) * SUL23-603: removed padding-left on branch name (#205) * SUL23-604: tweaking the toggle spacing (#206) * SUL23-573: tightening up spacing (#199) * SUL23-573: tightening up spacing * fixup * fixup for the hover on email and phone * fixup * adjusting the top and bottom spacing and removed empty spacers * fixup * SUL23-608: changed the padding-left when the browser size is smaller (#207) * fixup for the extra space * fixup * SUL23-605: fine tuning the sul people page. (#204) * SUL23-605: fine tuning * src/ * asking for feedback * Fix on click handlers * fixup to the group classes * adding hover and active * Added checkmark visual indicator --------- Co-authored-by: Mike Decker <[email protected]> * SUL23-601: changed when focused to not be red with red underline (#208) * SUL23-601: changed when focused to not be red with red underline * fixup * SUL23-601: fixed the All to All specialists per slack rquest. * SUL23-609: fix to text in hours card on homepage (#209) * SUL23-609: fix to text in hours card on homepage * fixup * Sul23 611 Improve and simplify toggles (#210) * Add a clear cache button for preview environments * SUL23-610 add SDR as an oembed provider in the wysiwyg * Sul23 611 tweaks (#211) * SUL23-611: draft work for spacing on small screens * adjustments to the focus and toggles * tweaks * tweaks * fixup * fixup * fixup * fixup * Removed old eslint file * SUL23-614 SUL23-615: fixes for html validation (#213) * SUL23-615: footer link size (#212) * SUL23-617: changed to Expertise (#214) * Updated changelog --------- Co-authored-by: pookmish <[email protected]> Co-authored-by: Jen Breese <[email protected]> Co-authored-by: Mike Decker <[email protected]>

SUL23-605: fine tuning

da68cdc

github-actions bot added the size/xs label Aug 15, 2024

vercel bot deployed to Preview August 15, 2024 22:52 View deployment

src/

4bc7efe

vercel bot deployed to Preview August 16, 2024 04:02 View deployment

asking for feedback

79687d8

vercel bot deployed to Preview August 16, 2024 16:49 View deployment

jenbreese commented Aug 16, 2024

View reviewed changes

Fix on click handlers

0eda833

vercel bot deployed to Preview August 16, 2024 18:02 View deployment

fixup to the group classes

8933c14

vercel bot deployed to Preview August 16, 2024 18:29 View deployment

jenbreese assigned pookmish Aug 16, 2024

pookmish requested changes Aug 16, 2024

View reviewed changes

pookmish removed their assignment Aug 16, 2024

adding hover and active

eb8e239

vercel bot deployed to Preview August 16, 2024 23:57 View deployment

jenbreese requested a review from pookmish August 17, 2024 00:08

jenbreese assigned pookmish Aug 17, 2024

jenbreese changed the title ~~SUL23-605: fine tuning~~ SUL23-605: fine tuning the sul people page. Aug 22, 2024

pookmish requested changes Aug 22, 2024

View reviewed changes

Added checkmark visual indicator

67e836e

github-actions bot added size/l and removed size/xs labels Aug 28, 2024

pookmish merged commit 703ae1d into 1.x Aug 28, 2024
4 of 5 checks passed

pookmish deleted the SUL23-605-specialists branch August 28, 2024 17:56

vercel bot deployed to Preview August 28, 2024 17:58 View deployment

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SUL23-605: fine tuning the sul people page. #204

SUL23-605: fine tuning the sul people page. #204

jenbreese commented Aug 15, 2024 •

edited

Loading

vercel bot commented Aug 15, 2024 •

edited

Loading

jenbreese Aug 16, 2024

pookmish Aug 16, 2024 •

edited

Loading

pookmish Aug 16, 2024

jenbreese Aug 16, 2024

pookmish Aug 16, 2024

jenbreese Aug 16, 2024

jenbreese Aug 16, 2024

pookmish Aug 16, 2024

jenbreese Aug 16, 2024

pookmish left a comment

jenbreese commented Aug 17, 2024

jenbreese commented Aug 21, 2024

pookmish Aug 22, 2024

jenbreese Aug 22, 2024

pookmish Aug 22, 2024

pookmish Aug 22, 2024

SUL23-605: fine tuning the sul people page. #204

SUL23-605: fine tuning the sul people page. #204

Conversation

jenbreese commented Aug 15, 2024 • edited Loading

READY FOR REVIEW

Summary

Review By (Date)

Criticality

Urgency

Review Tasks

Setup tasks and/or behavior to test

Site Configuration Sync

Front End Validation

Backend / Functional Validation

Code

Code security

General

Affected Projects or Products

Associated Issues and/or People

Resources

vercel bot commented Aug 15, 2024 • edited Loading

Choose a reason for hiding this comment

pookmish Aug 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pookmish left a comment

Choose a reason for hiding this comment

jenbreese commented Aug 17, 2024

jenbreese commented Aug 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jenbreese commented Aug 15, 2024 •

edited

Loading

vercel bot commented Aug 15, 2024 •

edited

Loading

pookmish Aug 16, 2024 •

edited

Loading