search

skeletonlabs / skeleton

A complete design system and component solution, built on Tailwind.
https://skeleton.dev
MIT License
4.89k stars 305 forks source link

Avatar (initials only) in AppBar rendering improperly in Safari #2408

Open fnimick opened 8 months ago

fnimick commented 8 months ago

Current Behavior

Adding an with initials rather than image (which was fixed in https://github.com/skeletonlabs/skeleton/pull/2333) causes the bar contents to render in the vertical center of the viewport.

The issue is the h-full class which was removed for image avatars, but not removed for text avatars.

See also: https://github.com/skeletonlabs/skeleton/issues/2055

Expected Behavior

No response

Steps To Reproduce

No response

Link to Reproduction / Stackblitz

No response

More Information

No response

endigo9740 commented 5 months ago

Hey @fnimick, sorry for the delay on this one. Can you confirm if the issue is still present? I'm unable to replicate:

Screenshot 2024-04-05 at 3 59 30 PM

I tested both with the src and initials, but neither cause the old issue to occur for me.

Note the version of Safari I'm running is listed in the screenshot. Safari versions are tied to the version of MacOS you're running. In my case I'm on Sonoma 14.4.

fnimick commented 5 months ago

@endigo9740 why not simply remove h-full from the classes generated in the initials case to match the image case? I can't think of a reason for them to be distinct - especially since they matched prior to the removal of h-full in the image case to fix that bug.

Even if new versions of Safari fix the problem, we still need to support old ones.

endigo9740 commented 5 months ago

Even if new versions of Safari fix the problem, we still need to support old ones.

I agree, which is why I asked if you could replicate on an older OS to confirm the issue is still present. I don't make assumptions that changes will fix things, I work on empirical data. But it would be helpful to have confirmation as I cannot easily roll back my operating system version.

I'll make the change next time I have a chance to work on v2 issues, otherwise I will accept a PR - which will come with a preview URL we can verify against.

fnimick commented 5 months ago

Ah, I understand. I've also updated and can't test with old safari versions anymore, unfortunately.