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

Signup: DSS: Show loading overlay during image preloading #1256

Merged
merged 6 commits into from
Dec 4, 2015

Conversation

sirbrillig
Copy link
Member

Fixes #1163

Testing

Load http://calypso.localhost:3000/start/dss/ and try searching for words. Make sure that the images load correctly and with very little or no flicker of unstyled previews.

Try the following types of search events to look for strange behavior:

  1. Type a search very slowly.
  2. After typing a multi-word search, press backspace several times to delete the last word.
  3. Clear the search field either with the 'X' or with select-all + backspace.
  4. Slowly type a search for one word (e.g., "favorite") and then add another word (e.g, "favorite meal").
  5. Search for one word, then search for a different word.
  6. Search for one word, then search for a different word, then repeat the search for the first word again.
  7. Search for one noun (e.g., "game"), then pluralize it (e.g., "games").
  8. Search for something that returns no results (e.g., "asdasdlkjasdljs").

@sirbrillig sirbrillig added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Dynamic Screenshots (DSS) labels Dec 4, 2015
@sirbrillig sirbrillig self-assigned this Dec 4, 2015
@sirbrillig
Copy link
Member Author

ping @kwight for review

@kwight
Copy link
Contributor

kwight commented Dec 4, 2015

SUH-WEET. Looks great. 👍

@kwight
Copy link
Contributor

kwight commented Dec 4, 2015

Oh @sirbrillig , you've replaced tabs with spaces in style.scss.

@sirbrillig
Copy link
Member Author

you've replaced tabs with spaces in style.scss.

@kwight the other way around. da405c5

@sirbrillig
Copy link
Member Author

Also, I found a race condition having to do with how ImagePreloader works: try typing games very slowly. The overlay sticks around forever. I'm working on that now.

@sirbrillig
Copy link
Member Author

There. That should fix it.

@kwight
Copy link
Contributor

kwight commented Dec 4, 2015

Hm, now I get a flash of the default screenshots right before the white overlay kicks in :| And yep, I hadn't pulled to see the tabs.

@sirbrillig
Copy link
Member Author

hm... now I'm seeing that too. back to research...

@sirbrillig
Copy link
Member Author

ok, I have a few possibilities I've tried, but they all flicker one way or another:

  1. Show the image previews behind the white overlay as they are loading.
  2. Show the default screenshot behind the white overlay any time loading is going on.
  3. Show an opaque overlay during any loading.

I think we need to decide what we want it to look like while the search is in-progress. Ideally I'd say we want no image changes while the search is in-progress, and no flickering as it renders the searched image, but what does that look like, exactly? How does the code know when the user has actually stopped typing? We may need to disable search-as-you-type to get it.

@sirbrillig sirbrillig force-pushed the fix/dss-overlay-during-preload branch from 1b46020 to cd4f71c Compare December 4, 2015 16:20
@sirbrillig sirbrillig force-pushed the fix/dss-overlay-during-preload branch from 5718685 to fcbbc76 Compare December 4, 2015 18:48
@sirbrillig
Copy link
Member Author

With the last commit I went with option 1 in my list above

Show the image previews behind the white overlay as they are loading.

It's actually looking pretty good.

@kwight
Copy link
Contributor

kwight commented Dec 4, 2015

With the latest changes, we don't go back to seeing the default screenshots while the results image loads in, but I prefer this. Seeing the new image load over the old image (and not confusing people by flashing the default screenshots) is a better experience.

@kwight
Copy link
Contributor

kwight commented Dec 4, 2015

LGTM 👍

@kwight kwight added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Dec 4, 2015
sirbrillig added a commit that referenced this pull request Dec 4, 2015
Signup: DSS: Show loading overlay during image preloading
@sirbrillig sirbrillig merged commit de5df67 into master Dec 4, 2015
@sirbrillig sirbrillig deleted the fix/dss-overlay-during-preload branch December 4, 2015 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants