launchpad-reviewers team mailing list archive

[Ines Almeida <mp+447200@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lib/canonical/launchpad/icing/style.css b/lib/canonical/launchpad/icing/style.css
> index 125f723..04dd339 100644
> --- a/lib/canonical/launchpad/icing/style.css
> +++ b/lib/canonical/launchpad/icing/style.css
> @@ -663,12 +692,18 @@ table.diff .line-no.active, table.diff .ss-line-no.active {
>    background: url(/@@/add) #f6f6f6 center left no-repeat;
>  }
>  table.diff .diff-chunk, table.diff .diff-file, table.diff .diff-header {
> - font-weight: bold;
> +  background: #f4f6f9;
> +  font-weight: 600;
>  }
> -table.diff .diff-added { background-color: #92ED92; }

The colors look a little bit too muted maybe? I'm afraid it might be hard to distinguish in some screens - in mine I feel like I have to make some effort. I actually like stronger colors here.

You mention in the commit message that it increases contrast - is it an accessibility thing?

> -table.diff .diff-removed { background-color: #FF7F7F; }
> +table.diff .diff-added { background-color: #e1ffe7; }
> +table.diff .diff-removed { background-color: #ffe6e4; }
>  table.diff .inline-comments > td > div {
> -  margin: 0 1em 1.5em 1em;
> +  margin: 0;
> +}
> +
> +table.diff .inline-comment {
> +    border-top: 1px solid #ddd;
> +    border-bottom: 1px solid #ddd;
>  }
>  table.diff .inline-comments .yui3-ieditor { padding-right:0!important; }
>  table.diff .inline-comments .yui3-ieditor-multiline .yui3-ieditor-btns
> @@ -676,7 +711,8 @@ table.diff .inline-comments .yui3-ieditor-multiline .yui3-ieditor-btns
>    word-wrap: normal;
>  }
>  table.diff .inline-comments .boardComment {
> -  margin: 1em 0;
> +  margin: 0.5rem;
> +  max-width: 43.75rem; /* 700px */

I'm not sure about the smaller comment box. I do understand that it makes it more similar to the main comments, but trying it, it feels a bit crammed to me (I might just be too used to how it was?)

I do quite like the new border separating the comment from the code :+1:

Also, any thoughts on increasing the padding within the comment box to give it more room to breathe?

>  }
>  table.diff .inline-comments .boardCommentBody {
>    word-wrap: break-word;


-- 
https://code.launchpad.net/~petermakowski/launchpad/+git/launchpad/+merge/447200
Your team Launchpad code reviewers is requested to review the proposed merge of ~petermakowski/launchpad:update-diff-view-ui into launchpad:master.