Code review comment for lp:~henninge/launchpad/bug-425583-languages

> Henning, this is great! The responsiveness of your JS solution is so much
> better than a page-load.

Thank you, I like it a lot, too. ;)

> UI-wise, at first I wasn't sure about having the language icon next to each
> language (as there is nothing to differentiate visually here), but then
> looking at other pages, such as:

Yes, you are right. It actually makes for clearer view code, too, using the tal formatter. Thanks for prodding me to do it, I was just to lazy to remember how to use a tal formatter in view code ...

> If I click on the link to edit my preferred languages, and update them, I
> expected to land back at this page, but it looks as though the preferred
> languages page always redirects back to the user profile page. That should be
> easy enough to fix (a next_url in the view which defaults to profile?).

I did that because I don't like things like that happening to me, either. But it was a bit more work because the view code was not handling the redirection correctly in the first place.

> Also, it should only be a few extra lines to call hide_and_show on a key press
> event for the input box (and hide the button). Do you want to give it a go, or
> did you decide against it for other reasons?

I tried it out now and it actually saves a few lines of js code but it is not very responsive. I'd have to do the hide_and_show asynchronously but I don't want to put that much work into it since searching for languages is not a daily task anybody would do (as apposed to say adding tags to a bug).

>
> IRC Notes:

I got the "unseen" class thing sorted out, it was shadowed by a more specific setting of display. I added an even more specific one. ;)

Thanks for the review, I think this really looks good now.