Launchpad itself

Code review comment for lp:~michael.nelson/launchpad/644328-blacklist-ui

644328-blacklist-ui
Merge into db-devel

Revision history for this message

Guilherme Salgado (salgado) wrote on 2010-09-23:

<salgado> noodles775, I like the graying out because it makes it easy to see everything that was changed at a glance and maybe undo, but the text (as well as the links) is a bit too verbose (IMHO) and don't allow switching between all different states, so I was thinking of using 3 radio buttons there. "Blacklisted: (*) No, ( ) This version, ( ) All versions"
<salgado> but that's more like a nice-to-have thing, not that important
<noodles775> salgado: that actually sounds a bit like what I'd proposed on the mockup (a single 'Add to blacklist' that would then give you those options), but henninge convinced me on a previous MP that it was a bit too much... hangon, I'll find the discussion.
<salgado> well, except that the radio buttons allow you to see all the possible states without having to click anything
<salgado> just like the links do
<salgado> but sure, maybe he'll be able to convince me as well. :)
<noodles775> salgado: ok - if you search for blacklist at https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2/+merge/34739 you'll see a bit of background.
<noodles775> salgado: but I think your radio button option you mention could work too... Also, if you've got suggestions for making the text less verbose while still being easy to understand, let me know :)
<noodles775> s/you/that you/
<salgado> what I like about the radio button is that you see all the options without having to click anything and there's no repetition
<salgado> the downside is that you need vertical real estate, but you seem to have plenty of that
<salgado> as for the text of the link, the only thing I can think of is something like "Blacklist: _this version_ or _all versions_"
<salgado> but I don't quite like that as it seems to suggest you need to blacklist something
<noodles775> salgado: Yeah. RE the radio buttons, I was going to say that it would require two clicks (ie select/submit), but it doesn't of course... we could do the api update when an option is selected quite easily. And that would mean no need for 'Undo'. I like it more and more :)
<salgado> indeed, I hadn't thought about the undo

<salgado> noodles775, I like the graying out because it makes it easy to see everything that was changed at a glance and maybe undo, but the text (as well as the links) is a bit too verbose (IMHO) and don't allow switching between all different states, so I was thinking of using 3 radio buttons there.  "Blacklisted: (*) No,  ( ) This version,  ( ) All versions"
<salgado> but that's more like a nice-to-have thing, not that important
<noodles775> salgado: that actually sounds a bit like what I'd proposed on the mockup (a single 'Add to blacklist' that would then give you those options), but henninge convinced me on a previous MP that it was a bit too much... hangon, I'll find the discussion.
<salgado> well, except that the radio buttons allow you to see all the possible states without having to click anything
<salgado> just like the links do
<salgado> but sure, maybe he'll be able to convince me as well. :)
<noodles775> salgado: ok - if you search for blacklist at https://code.edge.launchpad.net/~michael.nelson/launchpad/distro-series-difference-browser2/+merge/34739 you'll see a bit of background.
<noodles775> salgado: but I think your radio button option you mention could work too... Also, if you've got suggestions for making the text less verbose while still being easy to understand, let me know :)
<noodles775> s/you/that you/
<salgado> what I like about the radio button is that you see all the options without having to click anything and there's no repetition
<salgado> the downside is that you need vertical real estate, but you seem to have plenty of that
<salgado> as for the text of the link, the only thing I can think of is something like "Blacklist: _this version_ or _all versions_"
<salgado> but I don't quite like that as it seems to suggest you need to blacklist something
<noodles775> salgado: Yeah. RE the radio buttons, I was going to say that it would require two clicks (ie select/submit), but it doesn't of course... we could do the api update when an option is selected quite easily. And that would mean no need for 'Undo'. I like it more and more :)
<salgado> indeed, I hadn't thought about the undo

« Back to merge proposal