Show loading/error placeholder images when appropriate (closes #78).

mozilla / universal-search-content

the iframe that we put inside the autocomplete dropdown

7 stars 7 forks source link

Show loading/error placeholder images when appropriate (closes #78). #122

Closed chuckharmston closed 8 years ago

chuckharmston commented 8 years ago

This PR:

Shows a placeholder ellipsis image while favicons and preview images are loading, then removes it when loaded.
Shows a globe icon when favicons or placeholder images are missing or when they error.
Centers all favicons.
Adds margin to the magnifying glass icon so that aligns with favicons.

r? @bryanbell @6a68

When everything errors.

When everything is loading.

When everything has loaded.

bryanbell commented 8 years ago

Looks Great!

jaredhirsch commented 8 years ago

What if we just always show the globe during loading?

I don't think users will care about the loading status of the favicons, so flickering from dotted to globe might be more distracting than just showing a globe to begin with.

@bryanbell, @chuck, thoughts?

chuckharmston commented 8 years ago

That’s a fine idea. I’m on it.

chuckharmston commented 8 years ago

Refactored to remove the loading ellipsis icon. Since we're using the same icon for the loading and error states, we display the globe as the default state, show the icon if it successfully loads, and do nothing on error.

jaredhirsch commented 8 years ago

Sorry this review is dragging on, I think it's really close!

jaredhirsch commented 8 years ago

@chuckharmston Now that the view refactoring PR has landed, I think injectFavicon could pretty naturally be defined on the base row view. I'm happy to do the rebasing if you'd rather keep your brain in python-land, let me know

jaredhirsch commented 8 years ago

@chuckharmston Do you want to rebase this or close it for now?

chuckharmston commented 8 years ago

I'm pretty far away from working on this, I'll close it for now.