[PATCH 2 of 3 v2] diff: diff_block_simple: make it collapsable and default behaviour selectable

[Mads Kiilerich]

On 12/24/2015 09:32 PM, Jan Heylen wrote:
> # HG changeset patch
> # User Jan Heylen <jan.heylen at alcatel-lucent.com>
> # Date 1448811822 -3600
> #      Sun Nov 29 16:43:42 2015 +0100
> # Node ID 3bd79591517080ccf795e559650db057112b7219
> # Parent  f306d394f56a447d11521596df6ed730fbca9709
> diff: diff_block_simple: make it collapsable and default behaviour selectable

Ok, thanks. Now I can try it out and understand. I was missing that it 
was about making "diff_block_simple" (used in PRs and compare) behave 
more like "diff_block" (used in changeset and file diffs) and giving it 
the same collapse/expand options. (Please add something like that to the 
commit message.)

But while diff_block has one Collapse link for the diff of all involved 
files, this patch will give one link for each file (similar to the patch 
from us I linked to), and after collapsing one file, the expand text 
will be shadowed by the next collapse link. It has to work better with 
multiple files.

I am all for making these diff views more consistent and having he same 
amount of collapse/expand everywhere. I assume the different behaviour 
was unintentional and caused by confusing difference in the two very 
similar diff templates? It seems like the collapse link and container 
should live in pullrequest_show.html instead ... but that would also be 
weird. That very much suggests that it would be very nice to unify these 
two diff templates ...

(Btw: I wonder what the use case is for hiding all diffs in a changeset 
diff. Is it just to make it easy to scroll to the bottom of the page? (I 
can see more of a point about collapsing it on PRs if you want to 
encourage per changeset reviews. That wouldn't work for us right now ... 
but that is a different story.))

/Mads



>
> diff -r f306d394f56a -r 3bd795915170 kallithea/templates/changeset/diff_block.html
> --- a/kallithea/templates/changeset/diff_block.html	Thu Dec 24 21:28:19 2015 +0100
> +++ b/kallithea/templates/changeset/diff_block.html	Sun Nov 29 16:43:42 2015 +0100
> @@ -65,8 +65,21 @@
>   </div>
>   </%def>
>   
> -<%def name="diff_block_simple(change)">
> -
> +<%def name="diff_block_simple(change, hidden=False)">
> +<%
> +if hidden:
> +    container_class = "hidden"
> +else:
> +    container_class = ""
> +%>
> +<div class="diff-collapse">
> +    %if hidden:
> +    <span target="${'diff-container-%s' % (id(change))}" class="diff-collapse-button">↓ ${_('Expand Diff')} ↓</span>
> +    %else:
> +    <span target="${'diff-container-%s' % (id(change))}" class="diff-collapse-button">↑ ${_('Collapse Diff')} ↑</span>
> +    %endif
> +</div>
> +<div class="diff-container ${container_class}" id="${'diff-container-%s' % (id(change))}">
>     %for op,filenode_path,diff in change:
>       <div id="${h.FID('',filenode_path)}_target" style="clear:both;margin-top:25px"></div>
>       <div id="${h.FID('',filenode_path)}" class="diffblock  margined comm">
> @@ -124,6 +137,7 @@
>           </div>
>       </div>
>     %endfor
> +</div>
>   </%def>
>   
>   <%def name="diff_block_js()">