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

adding help text loop to loading page #917

Merged
merged 6 commits into from
Aug 11, 2019
Merged

Conversation

choldgraf
Copy link
Member

@choldgraf choldgraf commented Aug 7, 2019

This is a first pass at adding some helpful text that loops during the loading page. The goal is both to give users some information about what's currently happening, as well as to provide some interesting information about the project in general.

I think the list should never get too long - not sure exactly what that means, but this starts with a handful of messages to share with users as they wait. Over time, we can add and modify this.

Here's the text that will be updated in this PR. Note, it is sped way up just to show the messages w/o having a 60 second long gif, the actual seconds between message change is currently 4

hFgF7l0CmF

What do people think about this pattern for sharing information?

@choldgraf
Copy link
Member Author

cc @betatim who has been on a "designy" kick with me lately :-)

@betatim
Copy link
Member

betatim commented Aug 7, 2019

The fact that the log viewer changes position is :-/ (I think it already did when the error message appeared but this GIF really drives home the point about how annoying that is.)

Otherwise I like the idea. In scikit-learn/scikit-learn#11221 (comment) someone said "I think mybinder.org is broken for me", it turns out they misinterpreted the "taking longer than usual" as an error message. Maybe we can improve on that with this PR as well?

@choldgraf
Copy link
Member Author

I totally agree about the height. What do you think about:

Give enough whitespace for 2 lines on a 13" laptop, set fixed width for the "taking longer to load" message so we can better-predict the vertical space that the text takes up. Then just fill in the white space if the text messages spill over, and don't use text that takes up more than 2 lines?

re: the wording of taking longer than usual and people thinking it's broken. Do you think we should change that text, or change the "helpful messages" to add one about it?

@choldgraf
Copy link
Member Author

choldgraf commented Aug 8, 2019

OK, a few updates. Here's the latest version (again, with sped-up messages).

The following main changes:

  • There's now two sections to the "Starting repository" text - one that's static and bold, another that we update as the time gets longer
  • I increased the times needed for the messages, and made it so there are 3 in total instead of 2

uHSJEPeB5H

@betatim
Copy link
Member

betatim commented Aug 8, 2019

Screen Shot 2019-08-08 at 07 11 22

This is what it currently looks like. I wanted to have a comparison nearby.

Initial thought from "just woke up, dark laptop screen" Tim: is the grey/gray of the two sub messages dark enough to produce a high enough contrast? I thought it was tricky to read. We should check that.

Thoughts:

  • can we know if the repo is building or launching outside our log window? That would be super useful because we could then display "Building a new image for your repository, this can take a few minutes" instead of having to write the text as "maybe it is building, who knows"
  • one help message should be some good instructions about expanding the log window (or we make it expanded by default, which we had decided we should do in Upgrade xterm to 3.11.0 and remove vendored addon #787)
  • is it confusing if there are two messages that scroll independently? My feeling is that one change interval is easier for new users to understand. This means that we have a "large message" and a "small message" that both work together to communicate one idea. Then it changes to the next pair of "large message" and "small message" that communicate the next idea.
  • I like the extra white space

I think this will be a good improvement to the UI!


It also makes me think we should start the process of redesigning the UI elements taking into account all the things we learnt, features we added, and what a good UX looks like. We are on the brink of having a "messy" UI because we keep adding to it instead of designing it specifically with all the components in mind. Something for a new PR/issue.

@choldgraf
Copy link
Member Author

choldgraf commented Aug 9, 2019

OK, the latest push cleans up a few things per @betatim's comments - I thought about it a bit more on the plane and I agree with you, I think having only one thing change on the page is preferable. So, here's what this does:

  • The big text no longer changes
  • For the first N (where N is currently 120 but I'm flexible on this) seconds, the smaller message will rotate through the ones in this PR
  • After 120 seconds, the message freezes to the "this is taking a while to load" message

This way we keep the same general structure of the page, and avoid changing vertical spacing when new messages pop up. Here's a gif:

armIt4TnOY

(it'll be har to tell above because the GIF will loop, but trust me that the image stops changing after the "it's taking a while to start" image :-) )

return false;
}

// export entrypoints
window.loadingMain = loadingMain;
window.indexMain = indexMain;

// Show the logs by default
log.show();
Copy link
Member

Choose a reason for hiding this comment

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

Yay!

Copy link
Member

Choose a reason for hiding this comment

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

Just tried this locally and the log doesn't open :-(

While pondering how to fix it I replaced the "building" state handler (L174) like so:

  image.onStateChange('building', function(oldState, newState, data) {
    $('#phase-building').removeClass('hidden');
    log.show();
  });

this works and only open the log if we are building the repo, otherwise it stays closed. It is a bit weird if it takes a moment for the build to start because the log is first closed (waiting for build to start...) and then pops open. If that is too weird I'd insert the log.show() in image.onStateChange('waiting', function(oldState, newState, data) { which I think would open the log for everyone.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmmm - OK, well I assumed that would be an easy fix. If it isn't working and requires debugging, can we do this in a different PR? (I can just remove this change if so) I don't think it's strictly related to the image helper rotate thingy?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good to me!

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, removed in latest push! lemme know if you want me to iterate on the messages or timing or anything else!

@betatim
Copy link
Member

betatim commented Aug 11, 2019

Merging now. I think we can work on the messages in a new PR as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants