popcodeorg / popcode

An HTML/CSS/JavaScript editor for use in the classroom
MIT License
189 stars 139 forks source link

Chill out spinner/error messages #781

Closed outoftime closed 6 years ago

outoftime commented 7 years ago

Currently the spinner and error messages are quite visually intense, and can be very distracting for students who are trying to type code without constantly having their attention pulled to the right half of the screen. A few ideas for making this better:

One thing to note is that the spinner behavior is primarily a UX trick—the idea is to not show error messages immediately as you’re typing. There is no substantial delay before the outcome of code validation is available. The idea of the spinner is to reduce the annoyance of immediately being hit with error messages as you’re typing, but it’s not terribly effective given that students tend to type slowly and pause a lot.

This is extracted from a discussion in https://github.com/popcodeorg/popcode/issues/295

inlinestyle commented 7 years ago

@outoftime just a thought (also going back to https://github.com/popcodeorg/popcode/pull/771#issuecomment-294367332) - The loading message shown when loading from a Gist (looks like it's the <PopThrobber> component?) is a lot chiller than the spinner. Perhaps we could show that for loading stuff instead?

outoftime commented 7 years ago

@inlinestyle definitely agree that the throbber is the chillest, although I think for the “you’re typing and the code doesn’t currently validate” state, it was a mistake to have any sort of motion (my bad). I’m still pretty enamored with the “semi-transparent dark overlay over the preview of the last good state of the page” idea, although it would add a bit of complexity in maintaining the last good state of the page in application state.

As for the loading indicator you introduced in #771—realistically I would love to see the <PopThrobber> there but it would possibly be a bit of a pain to implement the throbber without React (and the nice SVG inlining we get for free etc.) so I didn’t want to let the perfect be the enemy of the good : )

inlinestyle commented 7 years ago

@outoftime 1) Makes sense. 2) You do lose the nice pre-made React component, but I don't think it's that much of a pain in absolute terms? The pulsing image could be implemented as a single CSS class combining background-image and animation, which could be applied to any given element. The exception would be if the fill color needs to be dynamic, but it doesn't look like the app currently requires that.

outoftime commented 7 years ago

@inlinestyle I’m convinced! If you feel moved to take a shot at it at any point, I’d very happily merge : )

inlinestyle commented 7 years ago

@outoftime I'd be happy to, I'll put it after https://github.com/popcodeorg/popcode/issues/302 on my todo list.

outoftime commented 7 years ago

awesome!!

outoftime commented 6 years ago

Fixed by #1049