Skip to content
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.

Sign up for GitHub

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

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

Merged
merged 7 commits into from
Aug 28, 2024
Merged

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

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
da68cdc
SUL23-605: fine tuning
jenbreese Aug 15, 2024
4bc7efe
src/
jenbreese Aug 16, 2024
79687d8
asking for feedback
jenbreese Aug 16, 2024
0eda833
Fix on click handlers
pookmish Aug 16, 2024
8933c14
fixup to the group classes
jenbreese Aug 16, 2024
eb8e239
adding hover and active
jenbreese Aug 16, 2024
67e836e
Added checkmark visual indicator
pookmish Aug 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 22 additions & 35 deletions src/components/views/sul-people/sul-people-table-view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,33 +33,17 @@ const SulPeopleTableView = ({items, hasHeading}: Props) => {
const id = useId()
const keywordRef = useRef<HTMLInputElement>(null)

const [typeFilter, setTypeFilter] = useState<string[]>([])
const [typeFilter, setTypeFilter] = useState<string>("")
const [keywordFilter, setKeywordFilter] = useState("")

let displayedItems = items

if (typeFilter.length >= 1) {
if (typeFilter) {
displayedItems = items.filter(
item => !!item.types?.map(type => type.toLowerCase()).filter(value => typeFilter.includes(value)).length
item => !!item.types?.map(type => type.toLowerCase()).filter(value => typeFilter === value).length
)
}

const updateTypeFilter = (type?: string) => {
if (!type) {
return setTypeFilter([])
}
setTypeFilter(prevState => {
const newState = [...prevState]
const existingIndex = newState.indexOf(type)
if (existingIndex >= 0) {
newState.splice(existingIndex, 1)
} else {
newState.push(type)
}
return newState
})
}

if (keywordFilter) {
displayedItems = displayedItems.filter(
item =>
Expand All @@ -75,7 +59,7 @@ const SulPeopleTableView = ({items, hasHeading}: Props) => {
className="mx-auto mb-32 flex w-fit flex-wrap justify-center gap-30 lg:flex-nowrap"
onSubmit={e => e.preventDefault()}
>
<div className="relative max-w-[350px] md:w-[435px]">
<div className="relative w-full md:w-[435px]">
<label className="pl-15 text-18 font-semibold leading-[23px]" htmlFor={id}>
Search by name, title, or subject
</label>
Expand Down Expand Up @@ -113,39 +97,42 @@ const SulPeopleTableView = ({items, hasHeading}: Props) => {
<span className="sr-only">Search</span>
</button>
</div>
<div className="self-end">
<div className="w-full self-end md:w-[435px]">
<fieldset className="mx-auto flex w-fit items-center rounded-full">
<legend className="sr-only">Filter by speciality</legend>
<label className="group hidden cursor-pointer border-r-0 md:block">
<label className="group cursor-pointer border-r-0 md:block">
<input
type="checkbox"
type="radio"
name="specialist"
className="peer sr-only"
checked={!typeFilter.length}
onChange={() => updateTypeFilter()}
checked={!typeFilter}
onChange={() => setTypeFilter("")}
/>
<span className="block rounded-l-full border border-r-0 border-black-80 p-9 px-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">
All specialists
<span className="block whitespace-nowrap rounded-l-full 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:text-16">
All
</span>
</label>
<label className="group cursor-pointer">
<input
type="checkbox"
type="radio"
name="specialist"
className="peer sr-only"
checked={typeFilter.includes("subject specialist")}
onChange={() => updateTypeFilter("subject specialist")}
checked={typeFilter === "subject specialist"}
onChange={() => setTypeFilter("subject specialist")}
/>
<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">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also FWIW, the contrast needs to be 3:1

Subject specialists
</span>
</label>
<label className="group cursor-pointer">
<input
type="checkbox"
type="radio"
name="specialist"
className="peer sr-only"
checked={typeFilter.includes("technical specialist")}
onChange={() => updateTypeFilter("technical specialist")}
checked={typeFilter === "technical specialist"}
onChange={() => setTypeFilter("technical specialist")}
/>
<span className="block items-center rounded-r-full border 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">
<span className="block items-center whitespace-nowrap rounded-r-full border 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:text-16">
Technical specialists
</span>
</label>
Expand Down
Loading