hypothesis / h

Annotate with anyone, anywhere.
https://hypothes.is/
BSD 2-Clause "Simplified" License
2.94k stars 427 forks source link

Use ArrowLeftIcon for groups back arrow #8899

Closed acelaya closed 1 month ago

acelaya commented 1 month ago

Use ArrowLeftIcon from frontend-shared for the groups form back arrow, and make it more vertically aligned via flex utilities.

Before:

image

After:

image

acelaya commented 1 month ago

It's worth noticing that the original ASCII arrow does not appears misaligned always. We have noticed it with some Linux default system fonts, but people using Mac doesn't have the same experience.

robertknight commented 1 month ago

Using flexbox like this aligns the center of the icon with the center of the inline box in which the font is drawn. That won't give consistent results across fonts because different parts of the font may be drawn at that point of the text box.

macOS old:

macOS old

macOS new:

macOS new

What I think might work better is to let <a> use its default inline layout and then use inline layout in the icon. This has the effect of aligning the bottom of the icon with the baseline of the font:

Baseline alignment
diff --git a/h/static/scripts/group-forms/components/CreateEditGroupForm.tsx b/h/static/scripts/group-forms/components/CreateEditGroupForm.tsx
index d877d7707..ef433c848 100644
--- a/h/static/scripts/group-forms/components/CreateEditGroupForm.tsx
+++ b/h/static/scripts/group-forms/components/CreateEditGroupForm.tsx
@@ -275,10 +275,9 @@ export default function CreateEditGroupForm() {
             <a
               href={group.link}
               data-testid="back-link"
-              className="flex items-center gap-0.5"
             >
-              <ArrowLeftIcon className="scale-75" />
-              Back to group overview page
+              <ArrowLeftIcon className="inline scale-75" />
+              <u>Back to group overview page</u>
             </a>
           )}
           <div className="grow" />

Can you give this a try on Linux and report what it looks like?

acelaya commented 1 month ago

Can you give this a try on Linux and report what it looks like?

This is how it looks on Linux

image

I think it's pretty good, so I'll push the changes.

EDIT: I zoomed in for the screenshot above. When I went back to 100% zoom, it stopped looking that good 😅

image


BTW, I noticed you added the underline to the text via <u />. Although it probably makes sense from an accessibility point of view, should we defer that and do that kind of change everywhere?

seanh commented 1 month ago

Testing this on macOS the vertical alignment looks exactly the same as on production/main:

Screenshot 2024-08-29 at 07 58 10 Screenshot 2024-08-29 at 07 58 18

I do think the arrow icon looks better than the unicode character. On the other hand, this will become a difference between this edit-group page and other pages in h that use the same unicode left arrow for this back link.

I'll leave it up to you guys.

acelaya commented 1 month ago

On the other hand, this will become a difference between this edit-group page and other pages in h that use the same unicode left arrow for this back link.

Yeah, I thought about this point.

Considering the inline solution still doesn't look properly aligned on Linux (see the second screenshot here https://github.com/hypothesis/h/pull/8899#issuecomment-2315482329), I'd suggest we don't merge this yet. We can focus on it as a global improvement at some other point, changing the icon/s consistently and spending a bit more time testing potential solutions for the vertical alignment that work in all systems.

robertknight commented 1 month ago

BTW, I noticed you added the underline to the text via . Although it probably makes sense from an accessibility point of view, should we defer that and do that kind of change everywhere?

Oh, sorry. That shouldn't have been in the diff. That was me testing where exactly the underline was drawn in relation to the baseline.