Code review comment for lp:~matthew.revell/launchpad/bug-heat-help-bug-544799

Revision history for this message
Michael Nelson (michael.nelson) wrote :

Thanks Matthew.

{{{
13:55 <mrevell> Something strange happened with my branch; I moved the styles, as you asked, and the underline returned. I moved them back and it's still there, so I've obviously done something without realising it. May I ask what the purpose of the move was?
13:56 <mrevell> Sorry, I'm getting called downstairs
13:56 <mrevell> bbl
13:56 <noodles775> Sure, we're trying to get rid of the old style.css, but we can only do it bit-by-bit, so we're starting with the general styles that we know are still in use.
--- Log closed Fri Apr 23 14:01:31 2010
--- Log opened Fri Apr 23 14:53:56 2010
14:53 -!- mrevellunch is now known as mrevell
14:54 <mrevell> Ah, I see
14:54 <mrevell> I'm gonna see if I can get this working again.
14:54 <noodles775> So that's strange that the underline returned. Did you try inspecting it (have you used firebug or chromium's developer tools? they're great for finding out what style is being applied)
14:55 <mrevell> I didn't use any tools, no. I'll give that a go, thanks. I'm using Chromium so will see what it can offer
14:58 <noodles775> Great, just make sure you've installed the package chromium-browser-inspector and then you should have the option available.
14:58 <mrevell> Aha
--- Log closed Fri Apr 23 15:03:31 2010
--- Log opened Fri Apr 23 15:35:38 2010
15:35 <mrevell> Hey, I've worked out what the problem is (I think). What I mentioned earlier about moving the style from one file to the other was a red herring. This issue seems to be that the default dotted border-bottom is applied to the <a> whereas the border: none is applied to the image, so the image doesn't have border but the anchor does.
15:36 <noodles775> Hrm.. so why was it working (or seemed to be working) earlier?
15:36 <noodles775> But that makes sense.
15:37 <mrevell> All I can think is that I'd not restarted my local instance (and so bugtask.py was still compiled with the inline style).
15:37 <noodles775> Yeah.
15:37 <noodles775> Question:
15:38 <noodles775> I noticed when looking at that branch, that the html generated from the browser *doesn't* include the class attribute (class="help"), where is it added?
15:38 <noodles775> I assume by some js somewhere?
15:38 <noodles775> Anyway, if it does it nicely, you should be able to add class="noborder" to the html generated in the browser code, and then update the style to:
15:39 <noodles775> a.help.noborder {....}
15:39 <mrevell> I believe so.
15:39 <noodles775> (no spaces in that selector).
15:39 <noodles775> Do you want to try that and see if it works?
15:39 <mrevell> Ah, thanks. I'll give that a go. thanks so much for your help
15:39 <noodles775> You can use the inspector to check that both classes are present when the page is generated (ie. it should be class="help noborder" or similar).
15:40 <noodles775> No problems!
--- Log closed Fri Apr 23 15:45:31 2010
--- Log opened Fri Apr 23 15:47:55 2010
15:47 <mrevell> That worked beautifully; thanks.
15:48 <noodles775> Excellent!
15:48 <noodles775> Oh,
15:49 <noodles775> Sorry, I just realised, that class="icon" would actually be more meaningful, rather than "noborder" (which is a presentation thing). Hrm, forget it, that's just too pedantic for friday afternoon :)
15:50 <mrevell> No worries, I have to make another commit to shift it back from style.css to style-3-0.css.in
15:50 <noodles775> ah ok, great.
}}}

review: Approve (ui)

« Back to merge proposal