fix(web): rating stars accessibility

[ben-basten]

Description

Fixes #11829

Fixes the user feedback from Discord

Accessibility improvements include:

• using native HTML radio buttons for star interactivity, which allows for predictable keyboard navigation
• only a single tab stop for the stars
• visible focus ring around focused star
• "Clear ratings" button to set ratings back to 0, if desired. According to the previous PR, it looks like there's no way to "reset" ratings
  • feat(web): add Exif-Rating #11580 (comment)
• add labels to each individual star and the grouping of stars, so the screen reader will announce what they are

Other improvements:

Changed the focusOutside Svelte action to also trigger if the relatedTarget is falsy. This means that scenarios like clicking outside of the browser window are handled, and it allows the action to function as a standalone equivalent to the clickOutside action.

The downside is that a tabindex needs to be added to the container that focusOutside is used with, to guarantee that all mouse clicks inside of the container have a relatedTarget when focus changes.

This is an important change to guarantee that the focus ring around the stars behaves like real focus, and never gets stuck on a star when the user clicks away from the immich window.

How Has This Been Tested?

• VoiceOver screen reader testing for Safari, Firefox, Chrome
• Test other usages of focusOutside Svelte action to check that they still work as expected

Screenshots (if appropriate):

No stars selected

Stars selected

[jrasm91]

Looks good to me. I think we talked about clearing actually sending back null instead of zero.

[stumpigit]

Thank you very much for these adjustments. We have already had some discussions about this in PR #11580 . I think these are clear improvements.
Would it also be possible to check that PR #11829 is taken into account? If the component is readonly, the hover effect should not work. That would be great.

[ben-basten]

Would it also be possible to check that PR #11829 is taken into account? If the component is readonly, the hover effect should not work. That would be great.

@stumpigit Thanks for bringing attention to this! Yes, the radio buttons are disabled and the hover animation is turned off in read-only mode. The setHoverRating method does this.

I indicated the #11829 will be fixed in the PR description.

[ben-basten]

I think we talked about clearing actually sending back null instead of zero.

@jrasm91 I totally agree - it would be preferred to be able to completely reset any changes made to the star rating, rather than setting it to 0.

When I try allowing nulls on the server, I'm seeing type errors from the ExifTool because Rating only allows numeric inputs:

immich/server/src/services/metadata.service.ts

Lines 289 to 299 in a37e7f0

	const exif = _.omitBy<Tags>(
	{
	Description: description,
	ImageDescription: description,
	DateTimeOriginal: dateTimeOriginal,
	GPSLatitude: latitude,
	GPSLongitude: longitude,
	Rating: rating,
	},
	_.isUndefined,
	);

I'm not sure how to resolve this, do you have thoughts about how to proceed? There's a similar comment about this on the previous PR:

#11580 (comment)

[jrasm91]

I think when star rating is null it needs to be deleted from the sidecar file itself. I'm not sure how exactly to do that. One option, if that isn't really doable, is to write zero, but map that to null when we read it and put it in the database

[ben-basten]

@jrasm91 I built a proof of concept with the nullable ratings. Here's a diff comparing it to the PR branch:

fix/ratings-a11y...ben/null-rating-poc

When the exiftool processes a null rating, it completely removes Rating from the sidecar file. This means that the ratings data will go back to being sourced from the original file, rather than being overwritten by the sidecar.

If the original file has a rating of null or 0, this is a non-issue. However, this becomes a confusing experience if the original file has a non-zero rating, and clicking the "Clear ratings" button repopulates the rating stars based on the original file's metadata.

With this in mind, I think that it makes the most sense to reset the stars with 0, because this most accurately matches the user intention when clearing ratings. It also means the empty rating would be maintained if a user ever moved their data to a different service.

What do you think about sticking with sending 0 to clear the ratings?

[jrasm91]

I am fine writing 0 to the sidecar to imply null, but I think we should have null in the database for search purposes.

Is it possible to:

• send null in the API
• write null to the database
• map null to 0 when writing tags
• map 0 to null when reading tags

[ben-basten]

Thanks for the context, that's very helpful! I haven't explored the server code much, so I'll do some digging to find a solution for this.

[jrasm91]

Thanks for the context, that's very helpful! I haven't explored the server code much, so I'll do some digging to find a solution for this.

I'm happy to implement the server changes if you want. Just let me know. I can always do it in a sister PR as well.

[ben-basten]

I'm happy to implement the server changes if you want. Just let me know. I can always do it in a sister PR as well.

Yes, I will take you up on that. Whether it's in a separate PR or not is ok by me - if you do a sister PR, I can update the frontend to send nulls this weekend.

Thanks for the help!