Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in

Issue 566920043: Issue 5578: Add a button to flip between old and new regtest images (Closed)

Can't Edit
Can't Publish+Mail
Start Review
7 months, 3 weeks ago by Dan Eble
7 months, 2 weeks ago


https://sourceforge.net/p/testlilyissues/issues/5578/ LIVE DEMO: http://faithful.be/tmp/test-results/ If JavaScript is enabled, a "Flip" button appears under the right-hand image. While the button is held down, the right-hand image is replaced by the left-hand image. Flipping back and forth can make small differences more obvious than static highlighting does. The output-distance script used to use HTML tags that are unsupported in HTML5, such as <tt> and <font>. The new approach uses CSS for general styling as well as for the flipping feature. The new style is not exactly the same as before, but should look familiar. The style sheet also includes a dark color scheme which newer browsers may select based on user preference. I found the program "tidy" to be somewhat useful for catching problems like empty or unclosed tags in the generated HTML, and I added it to the build as an option. http://validator.w3.org caught a few more problems, but tidy is fast and I didn't see it raise any incorrect objections, so it seems beneficial to use it when it is available.

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+175 lines, -63 lines) Patch
M GNUmakefile.in View 1 chunk +7 lines, -1 line 0 comments Download
M config.make.in View 1 chunk +1 line, -0 lines 0 comments Download
M configure.ac View 1 chunk +4 lines, -0 lines 0 comments Download
M scripts/build/output-distance.py View 12 chunks +163 lines, -62 lines 1 comment Download


Total messages: 3
Dan Eble
7 months, 3 weeks ago (2019-10-19 01:06:34 UTC) #1
Very nice, thanks! https://codereview.appspot.com/566920043/diff/581180043/scripts/build/output-distance.py File scripts/build/output-distance.py (right): https://codereview.appspot.com/566920043/diff/581180043/scripts/build/output-distance.py#newcode1030 scripts/build/output-distance.py:1030: open_write_file (dest_dir + '/style.css').write(''' Wouldn't it ...
7 months, 3 weeks ago (2019-10-19 06:08:51 UTC) #2
Dan Eble
7 months, 3 weeks ago (2019-10-19 12:43:53 UTC) #3
On 2019/10/19 06:08:51, lemzwerg wrote:
> Very nice, thanks!

Thanks for all your reviewing time lately.

> Wouldn't it be better/more convenient/more compact to write the CSS stuff into
> the HTML file, too?

The style is also used for the "details" files, e.g.


So I think it makes more sense to leave it external.

> Otherwise I can imagine to provide `style.css` in the
> source tree, to be not generated at all.

We could probably say the same for the JavaScript.  Viewing things as they are,
that's not a bad idea; however, there was a point during this task where I
almost introduced differences in the CSS based on the --no-compare-images
option.  I decided I had more useful tasks to get on with, but I don't want to
move the CSS to its own file and learn later that it wasn't well considered.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b