© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
Henning, this is great! The responsiveness of your JS solution is so much better than a page-load. I'm only marking it as needs fixing because of where you end up if you edit preferred languages.
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:
https:/
we display person icons for every member (although there are a few team icons thrown in there to justify it). What are your thought? I'm tending towards leaving it (even if we don't need to differentiate visually between the languages, the icon looks nice and is consistent with the rest of LP).
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?).
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?
IRC Notes:
<henninge> noodles775: Guten Morgen! ;)
<noodles775> Hi hennige!
* noodles775 has changed the topic to: on-call: noodles775 || reviewing: - || queue [] || This channel is logged: http://
<noodles775> :-D
<henninge> noodles775: ah, you are the ocr! :)
<noodles775> Yeah, myself and al-maisan are around today.
* al-maisan will be around after preparing his cappuccino <wink> :)
<henninge> noodles775: cool, let me prepare an mp for a final ui review. I'd like to have your input on the languages page.
<noodles775> Wow - that was fast?
<henninge> noodles775: well, I did not go for using and improving the lazr-js widget, I am sorry.
<henninge> noodles775: but I quite like what's come out of it and I only had yesterday to do it. ;)
<noodles775> henninge: no, I didn't expect you to do that as part of the initial branch (it's not necessary for the improved behavior, it'd just be a nice bling later).
<henninge> noodles775: It *does* have the client-side filter, just not as-you-type. You need to click.
<henninge> oh, and windmill test ist still missing ...
<noodles775> Aha.
* adeuring (<email address hidden>) has joined #launchpad-reviews
<henninge> noodles775: ok, mp is done.
<henninge> noodles775: I'd ask you to please branch it and have a look at the page.
<noodles775> Great, yep, I always do :-)
* noodles775 has changed the topic to: on-call: noodles775 || reviewing: - || queue [henninge/
<henninge> noodles775: also, I am not sure if I am writing good js/yui code.
<noodles775> OK, I'll check it out too.
<henninge> noodles775: fyi, just pushed a new version.
<noodles775> henninge: is this dependent on other changes currently in db-devel only? Or why are you targeting db-devel?
<henninge> noodles775: well, I just don't want to get caught in yui3final changes by working against the latest version.
<henninge> noodles775: Also, after the roll-out (when this lands) db-devel will have become the new devel. So eventually it will be merged into devel.
<noodles775> ah, I thought the yui3final stuff was to be merged back into devel by now, maybe it wasn't (or I misunderstood). But yes, +1 for landing this on devel (so it hits edge) after the roll-out.
* bigjools (n=quassel@
* al-maisan has quit (Read error: 104 (Connection reset by peer))
* al-maisan (<email address hidden>) has joined #launchpad-reviews
* bigjools has quit (Remote closed the connection)
* al-maisan has changed the topic to: on-call: noodles775, al-maisan || reviewing: -, - || queue [henninge/
* bigjools (n=quassel@
* noodles775 has changed the topic to: on-call: noodles775, al-maisan || reviewing: henninge/ui, - || queue [henninge/code] || This channel is logged: http://
* mrevell (n=matthew@
<noodles775> henninge: wow - I didn't realise that you had used the picker for the filtering! It should be really easy to update what you've done to filter on keypress.
<henninge> noodles775: beg your pardon?
<noodles775> henninge: I didn't realise that you'd actually done the filtering via JS - I was expecting the normal GET to +languages?blah
<noodles775> It's great!
<henninge> noodles775: Ah! Yes, that was fun I had after hours yesterday ... ;)
<henninge> noodles775: oh, I just read about the "unseen" css class.
* jelmer (n=jelmer@
<henninge> windmill doc is out-of-date ... :(
<noodles775> which one?
<noodles775> or you mean the actual documentation?
<henninge> https:/
<henninge> I read about the "unseen" class there but it does not seem to exist. But I am still checking.
<noodles775> BTW: I think you'd be best creating JS unit-tests for your new module...
<noodles775> (you'd still need one very basic windmill test though, hrm, so maybe it's easier just to do the windmill test for the moment).
<noodles775> henninge: also, all_languages should be a module-level var I think? (ie. so it's not evaluated each time hide_and_show is called)?
<henninge> noodles775: yes, sounds reasonable.
<henninge> noodles775: the "unseen class does exist in style.css". Is that not generally available?
<henninge> I could just move it to style-3.0.css
<noodles775> I'd be careful - Curtis is landing a branch very soon that removes a lot of the old styles... so yes, perhaps move it but be prepared for conflicts.