Code review comment for lp:~rharding/juju-gui/browser_links
Revision history for this message
| Richard Harding (rharding) wrote | # |
© 2004
Canonical Ltd.
•
Terms of use
•
Data privacy
•
Contact Launchpad Support
•
Blog
•
Careers
•
System status
•
0e1f616
(Get the code!)
Feedback below.
https:/
File app/subapps/
https:/
app/subapps/
On 2013/04/19 15:36:54, curtis wrote:
> Why undefined? I expect undefined to be a state that has never been
> examined/set. Why isn't null correct?
Null would completely work. I can update it.
https:/
app/subapps/
On 2013/04/19 15:36:54, curtis wrote:
> Again, why define the value to be undefined? I think we mean the value
is null.
Will change
https:/
app/subapps/
Y.one('
On 2013/04/19 15:36:54, curtis wrote:
> I think setStyle('display', 'assumed-
done in
> several places. We should be adding/removing classes. While 'none' is
valid for
> all html elements, 'block' is only valid for a subset of elements. The
design
> might need inline or inline-block in the future to make content
visible -- many
> code changes are needed where I would prefer huw or myself to make a
single
> style change to a class.
This is a bit of a temp hack since we're not the default, but out div
exists. I can XXX this to remove the whole thing going forward.
The other thing is that we have the collapsed view that's not here yet
that'll replace the job this is doing. I started to use the .hidden but
that uses visibility so our div is still there and 350px wide causing
click issues on the environment.
https:/
app/subapps/
content.
On 2013/04/19 15:36:54, curtis wrote:
> This is not an else. I can render search and editorial according to
this logic.
Thanks, missed this in refactors.
https:/