On Thu, Dec 24, 2015 at 5:55 PM, Mads Kiilerich <[email protected]> wrote: > On 12/07/2015 07:20 PM, Jan Heylen wrote: >> >> # HG changeset patch >> # User Jan Heylen <[email protected]> >> # Date 1448811822 -3600 >> # Sun Nov 29 16:43:42 2015 +0100 >> # Node ID eecdf2feaa98d065981b7b25f99e6843409f4f75 >> # Parent 543b35dff5e737398ae6eaac0ca98c857c5655fc >> diff: diff_block_simple: make it collapsable and default behaviour >> selectable > > > I don't see exactly how this should work and can't test it so I can't > comment much. Huh?
> > We use something similar - > https://bitbucket.org/Unity-Technologies/kallithea/commits/89ce15150e3e8c1b3acd0eeeb4cad0c97ab8f686 > . How do you think they compare? I don't know, this is the way it works in diff_block, the one used in collapsing commit diffs. > > I can see how it could be a good idea to not show the big diff ... but I > think it would be essential that it also didn't send the diff to the client > before it was shown. Perhaps a bit like the PR creation page loads the list > of changesets dynamically. (I think it would be even better if it somehow > could show the diffstat but expand it to the full diff.) Yes, that would be good, but this is not how this code works today, I only made sure the simple diff version, used in PR, also supports the collapsing thing, it is not my intention to change the behavior of the collapsing feature. > >> diff -r 543b35dff5e7 -r eecdf2feaa98 >> kallithea/templates/changeset/diff_block.html >> --- a/kallithea/templates/changeset/diff_block.html Thu Dec 03 >> 08:59:26 2015 +0100 >> +++ b/kallithea/templates/changeset/diff_block.html Sun Nov 29 >> 16:43:42 2015 +0100 >> @@ -65,8 +65,19 @@ >> </div> >> </%def> >> -<%def name="diff_block_simple(change)"> >> - >> +<%def name="diff_block_simple(change, hidden=False)"> >> +<% >> +if hidden: >> + expand_collapse_msg = "↓ ${_'Expand Diff')} ↓" >> + container_class = "hidden" >> +else: >> + expand_collapse_msg = "↑ ${_'Collapse Diff')} ↑" >> + container_class = "" >> +%> >> +<div class="diff-collapse"> >> + <span target="${'diff-container-%s' % (id(change))}" >> class="diff-collapse-button">${expand_collapse_msg}</span> > > > expand_collapse_msg gets extra quoting - it doesn't work as it is. > >> +</div> >> +<div class="diff-container ${container_class}" id="${'diff-container-%s' >> % (id(change))} > > > This is missing an ending '>' ... so it doesn't work and is untested? I did tested it, I cleaned/refactored the last part just before publishing so probably that's were it's gone missing, I'll update to V2, sorry for the confusion. > >> %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 +135,7 @@ >> </div> >> </div> >> %endfor >> +</div> >> </%def> >> <%def name="diff_block_js()"> >> _______________________________________________ >> kallithea-general mailing list >> [email protected] >> http://lists.sfconservancy.org/mailman/listinfo/kallithea-general > > _______________________________________________ kallithea-general mailing list [email protected] http://lists.sfconservancy.org/mailman/listinfo/kallithea-general
