-
Notifications
You must be signed in to change notification settings - Fork 638
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update diff2html to version 3 #1273
Conversation
4e6dbba
to
cee17da
Compare
Are there no changes required to the CSS? |
public/less/d2h.less
Outdated
background: transparent; | ||
} | ||
|
||
/*** adjustments for ungit ***/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything above this line is copied from https://github.com/rtfpessoa/diff2html/blob/4e28c5c3275f6f703a6923d23d001bfb676331a6/src/ui/css/diff2html.css
Everything below this line is customized for ungit.
Maybe we should split these into separate files.
public/less/d2h.less
Outdated
* | ||
* Diff to HTML (diff2html.css) | ||
* Author: rtfpessoa | ||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add the version number of diff2html here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, this part is copied 1:1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opted to include the css file from npm package so this file now only contains the adjustments.
Thanks for the comparison images. |
39ee3a9
to
292be99
Compare
f75e1fb
to
292be99
Compare
The height difference came from the different font (monospace) and font-size (13px) which I have changed back to the previous values here 14a6e9b Some other things which changed:
|
public/less/d2h.less
Outdated
-moz-user-select: none; | ||
-ms-user-select: none; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These don't seem to be necessary any more:
https://developer.mozilla.org/en-US/docs/Web/CSS/user-select#Browser_compatibility
Also, is this not done by diff2html already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only in the diff2html UI which we are not using. Also it seems a bit buggy. I have opened rtfpessoa/diff2html#303 for that.
3e8020e
to
18107b2
Compare
just to keep up with the API changes.
See rtfpessoa/diff2html#297 (comment)