launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Thanks for reviewing.

> 
> IE 8 (and 7) does not support :not(). I think you need to write this like this:
>     span.update-in-progress-message {}
>     button.update-in-progress-message:after {}
>

Ok, thanks. I struggled to get this right. I also recently added :not()
to formoverlay-core.css so will have to revisit that to try and fix it.


> I think the inline style on 402 is wrong. "style="max-width: 60em;" is only used when we know there is no side bar. sibling elements are 45ems. Why does this element allowed to extend beyond their length?
> 

The intent here was to limited the horizontal width of the message so
that it didn't expand out to totally fill the entire line if the browser
window were wide. ie long horizontal text is harder to read so I thought
we needed to have "Blah blah blah....".

> I see this pattern:
>      +            '    class="sprite remove action-icon remove-duplicate-bug">',
>   415+            '    Remove</a>',
> which puts leading white-space between the start of the anchor and the text. Even though the links are action-icon, parts of the underline can appear in some browsers, and quite likely for users that increase the text size. Maybe this is safer
>      +            '    class="sprite remove action-icon remove-duplicate-bug">',
>   415+                 'Remove</a>',
> 

Bah, typo. Thanks.

> TAL process its instructions in a specific order, define, then condition. This also means that define is always called on the element. Even though you wrote the condition first, the define was already executed:
>   570+             tal:condition="context/bug/duplicateof|nothing"
>   571+             tal:define="duplicateof context/bug/duplicateof">
> Since we know the order we could write this:
>   570+             tal:define="duplicateof context/bug/duplicateof"
>   571+             tal:condition="duplicateof|nothing">
> 
> 

Good pickup. I missed this, thanks.

-- 
https://code.launchpad.net/~wallyworld/launchpad/improve-dupe-bug-ui-227310/+merge/118738
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.