search

allenai / allennlp-guide

Code and material for the AllenNLP Guide
https://guide.allennlp.org
Other
86 stars 41 forks source link

Chapter sections move the screen around #87

Open matt-gardner opened 4 years ago

matt-gardner commented 4 years ago

Feb-26-2020 15-56-57

The way this moves the frame around when you click on the expand button, instead of just smoothly expanding or collapsing below, is not as satisfying as the animation on the main screen. Is there an easy way to make this behave the same way as on the main screen, while retaining the link behavior?

matt-gardner commented 4 years ago

As I play with this more, it seems like closing a section always takes you up to the top of the chapter page, and expanding it always makes the expanded section move to the top of the screen. And clicking the "next" button is the same as closing one section then opening the next. I think it would be preferable to have the close and expand not move the screen, but I'm not really sure what "next" should do if we do that.

matt-gardner commented 4 years ago

I'm probably ok just removing "next" if we change this expand and close behavior. I also think it's fine to have multiple sections expanded. But we do want stable links to individual sections.

And this is just final, small, polish issues. The current behavior is ok, it just could be a bit better, if it's relatively easy to make it that way.

aaronsarnat commented 4 years ago

@matt-gardner I agree that having a smooth, animated experience would be more satisfying, so I hope it won't be too much of a disappointment if we don't address these UX changes before I roll off the project. I took a look at the request, and the potential solutions are more complicated than it might seem at first glance.

Though, I did address the issue where closing exercise auto-scrolls to the top. Fix is here: https://github.com/allenai/allennlp-course/pull/100.

The problem with smooth animation of expand/collapse behavior (similar to home page) in this case is that the current mechanics allow for only one panel to be open at a time. Thus, if any panel is open then clicking another panel would result in two panels animating simultaneously. This creates an unpredictable end scroll position. I.e. if you click on something you may end up scrolled to a weird part of the page when the dust settles, depending entirely on how tall each panel was. Figuring out an intuitive animated scroll behavior to mitigate this problem would be tricky.

You mentioned the possibility of allowing multiple panels to be open at once and not auto-scrolling at all. This would be possible, but would require some refactoring and some thinking around how hashes would work if active panels are no longer exclusive. I.e. if you have 3 panels open, what's in the URL?

As-is, this exercise expand/collapse/scroll behavior is basically the same as what was there when I started working on the project. I think you're right in that there are likely opportunities to improve exercise interaction, but I guess I took the original functionality for granted, as I was focused on tackling the items we had called out early on.

I discussed briefly with @carissas, and she was in favor of calling this state a good stopping point (which works for me, as I'm definitely ready to start my break...). Still, I hope that overall you're happy with where we ended up! 🙂

This project has been a lot of fun for me, and it's been a pleasure working with you guys! I hope this course app turns out to be a hit for the AllenNLP community when it gets released!

matt-gardner commented 4 years ago

No problem, thanks for all of your work on this! I'm really happy with how it turned out.

aaronsarnat commented 4 years ago

Very glad to hear that! 😄 I hope we get the chance to collaborate on another project at some point!