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.

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

Feat/avatar #6

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

Mahmoud-zino
Copy link
Sponsor

Note:
only svelte is implemented as @endigo9740 requested.
I didn't implement click and keyboard events because of a11y.

Copy link

vercel bot commented Jan 6, 2024

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

Name Status Preview Comments Updated (UTC)
skeleton-next-prototype ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 14, 2024 7:26pm

@endigo9740

This comment was marked as outdated.

src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
@Mahmoud-zino
Copy link
Sponsor Author

Mahmoud-zino commented Jan 6, 2024

@endigo9740
We have a very serious issue with the fallback in svelte. onerror is not being dispatched anymore if the image was not found, you can find the issue here: sveltejs/kit#8271

I tried Rich's suggestion to check the natural-width and height of the image but then if the image is not loaded on initialization (we are getting images from some-website usually) the width and height will be 0 so the src will be replaced with the fallback.

for now I've gone ahead and deleted the fallback props, but we should decide if we want to handle this inside the avatar or just ignore it.

@endigo9740
Copy link
Contributor

@Mahmoud-zino that's weird. Hmm, well something to monitor as Svelte 5 progresses. Maybe they'll resolve this?

Copy link
Contributor

@endigo9740 endigo9740 left a comment

Choose a reason for hiding this comment

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

Just a few things!

src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/components/Avatar/Avatar.svelte Outdated Show resolved Hide resolved
src/pages/components/avatars/PreviewSvelte.svelte Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants