search

ReviewNB / support

Issues and feature requests for ReviewNB
https://reviewnb.com
59 stars 8 forks source link

Diff design options. #26

Closed muarvyn closed 5 years ago

muarvyn commented 5 years ago

Thanks for a good job!

I haven't noticed any options to adjust the look of a notebook diff. In particular,

  1. the colours to highlight additions/removes/modifications look to me near indistinctive from unmodified text;
  2. I noticed only one way to show the diff: two panels side-by-side. How about other layouts, say, alike an output, generated by git diff? It would be nice to have a number of options to adjust the look of a notebook diff. Thanks.
amit1rrr commented 5 years ago

the colours to highlight additions/removes/modifications look to me near indistinctive from unmodified text;

Thank you for reporting this. We've been using GitHub highlighting colors (because they always felt apt to me), but forgot to counter in one difference. I.e. GitHub diff is always over white background whereas our notebook code cells diff is over grey background which makes it slightly hard to spot differences.

I have gone ahead and updated highlight colors so that they are more prominent for everyone. Let me know what you think.

I noticed only one way to show the diff: two panels side-by-side. How about other layouts, say, alike an output, generated by git diff?

Is there a strong use case for git diff or other diff layout styles? I feel it's a nice-to-have feature for small changes (which anyway take very short time to review).

On the layout front, I do plan to make left hand side of diff collapsible so newly added changes (right side) takes up the full screen space. This makes it easy to review and write comments when notebook is newly added or when you only care about the current version of the notebook. Here's the issue for it.

amit1rrr commented 5 years ago

Closing this for lack of activity. We have updated diff colors to be more prominent. And there's a separate issue for collapsible LHS.

Apart from these, I do not think there should be any other diff design option per se.