diffux / diffux Goto Github PK

View Code? Open in Web Editor NEW

255.0 255.0 15.0 1.55 MB

Perceptual diffs of responsive screenshots made simple.

License: MIT License

Ruby 75.62% JavaScript 6.46% CSS 4.48% HTML 13.44%

diffux's People

Contributors

Stargazers

Watchers

Forkers

lencioni lostmarinero tdooner hellobelda ang silky flarnie nebs menor svaj metaltoad kalw iq-scm

diffux's Issues

Snapshots that have a baseline should not be "under review" until after diff has been generated

I think this would be best solved by splitting up the snapshotting and diffing processes into separate workers, and giving snapshots more granular pending states. Probably:

Waiting
Taking snapshot
Creating diff (if baseline exists)
Under review
Accepted/rejected

Other states that snapshots might be in:

Deleted (likely not shown in UI)
Error (this concept currently does not exist in the app)

Remove `active` concept and cron job

My initial thought with this was to have a cron job run every ten minutes to create snapshots for all Urls. Since we are moving to a project+sweeps approach, this whole thing doesn't make any sense. We should clean it up so that we are in a better position to introduce the new stuff.

Detect and ask to fix URLs that are redirects

Some URLs always result in redirects. For example, if a site is HTTPS only, requesting the non-HTTPS URL will often result in a redirect to the HTTPS URL. Time could be saved on every request if the URL was updated to avoid the redirect. The app should be able to determine if this type of thing is happening, tell the user about it, and prompt them about fixing it. Updating the URL in this way should preserve the previous snapshots.

Replace Cloudinary with a local solution

Uploading the snapshot images to Cloudinary is one of the slowest parts of the snapshot process. Soon we will be able to trigger sweeps of snapshots on entire projects, at which point the slowness of the snapshotting process will become much more noticeable. To make sure that this process is as fast as possible, we should replace Cloudinary with something local that doesn't require sending bytes across the Internet.

Warn if snapshot image is bigger than the viewport width

A common issue with responsive sites is that an element is wider than the window width and breaks the layout (makes it wider). We could detect this and reject the image if we find out that it happened.

Needs to be more resilient against deleting URLs

Start a sweep
Before sweep has finished, delete some URLs from project

Some of the sweep workers will error, but the snapshots are left dangling. If you visit one directly, you'll see an error like:

undefined method `project' for nil:NilClass

Add client-side image comparison modes

Borrowing inspiration from Github, 2-up, Swipe, Onion Skin, and Difference would be useful ways to explore image differences.

Show just the differences with context, click to see entire image

When looking at a diff in code, you often just see the bits that were changed, possibly surrounded by some configurable amount of lines of context. This allows you to quickly see just the bits that have changed without needing to scroll through the entire file. In some code review tools (such as Gerrit), you also have the option to click to get more context or see the entire gap between diffs.

Similarly, it would be great to do this for our image diff. We could likely record the "rows" in the image that contain diffed pixels, and then use that to render just those sections of the diffed image for review. Throw some JavaScript in the mix to allow the user increase the amount of visible context while reviewing.

Auto-approve snapshots with 0% difference

This will cut down on a lot of the noise caused by using this tool.

Add i18n

Since @trotzig knows Swedish, that could be a great starting point to internationalize the app, but any language will be great.

Add Code Climate badge to readme

Add Contributing section to readme

It would be great to have some instructions that guide people who want to contribute to the project in the right direction.

After #35 is complete, a good thing to include in this section would be to provide translations either via code or just text in an issue.

Snapshots are occasionally truncated

Every so often I will see a snapshot that is missing the bottom bit of the page. Not sure why this is happening yet.

Add projects to collect groups of Urls and viewport widths

We want a model that represents a collection of URLs. A good name for this might be "Project" or more generically "Group". Then a Sweep operates on a Project and simply runs Snapshots for each Url in the Project. That way Diffux could be used easily for multiple projects (e.g. Causes, Trackmail, and Stars).

Viewport widths should belong to Projects, not Urls. This will allow us to more easily manage a Project's Urls.

Soft delete snapshots with no confirmation, add undo

Emulate user agent strings for viewport sizes

Some websites serve different content to specific user agents. It might not be a bad idea to have a few user agent strings that we use for various ranges of viewport widths. For instance, if the viewport width is near 320, we could set the user agent to match a phone.

An example of how to set the user agent string for PhantomJS can be found at: https://github.com/ariya/phantomjs/blob/master/examples/useragent.js

As I think about this more, we may also want to eventually add other properties to the Viewport model (e.g. orientation, pixel density, etc.), which would make it more like a Device model to more closely emulate whatever device characteristics.

Snapshots without viewports still appear on project page

Create a project with some viewport widths
Take some snapshots
Delete one of the viewport widths

Snapshots are sill accessible, but are missing some information, such as the viewport. We probably should just not show these, or delete them when the viewport is deleted.

Increase test coverage

This project is slowly graduating from a proof-of-concept and we should treat it as a serious thing. Specs should be added to cover the critical behavior.

Change concept of baseline to approve/reject

Setting the baseline image doesn't feel very intuitive. It would be a better experience to approve or reject snapshots, and the baseline represents the latest approved snapshot.

Add Travis CI

Once we get tests working, it would be useful to have Travis CI run the tests for us automatically. When we get Travis CI running, we should add a badge to our readme.

Pages need better titles

All of the page titles in the app are currently identical. However, every page should have a short, unique, and descriptive page title for basic usability.

No such file or directory @ rb_sysopen - ...

This happens occasionally when creating snapshots. Stacktrace:

Errno::ENOENT - No such file or directory @ rb_sysopen - /var/folders/qd/dqz3r9ns4m78x7b40t9j_jnh0000gn/T/d20140202-36949-56ko12/SQWNWHRO.png:
  cloudinary (1.0.67) lib/cloudinary/uploader.rb:56:in `block in upload'
  cloudinary (1.0.67) lib/cloudinary/uploader.rb:202:in `call_api'
  cloudinary (1.0.67) lib/cloudinary/uploader.rb:49:in `upload'
  app/services/file_util.rb:11:in `upload_to_cloudinary'
  app/services/snapshotter.rb:38:in `block in take_snapshot!'
  app/services/file_util.rb:6:in `block in with_tempfile'
  /Users/joelencioni/.rbenv/versions/2.1.0/lib/ruby/2.1.0/tmpdir.rb:88:in `mktmpdir'
  app/services/file_util.rb:4:in `with_tempfile'
  app/services/snapshotter.rb:16:in `take_snapshot!'
  app/controllers/snapshots_controller.rb:12:in `block in create'
  activerecord (4.0.2) lib/active_record/relation/delegation.rb:13:in `each'
  app/controllers/snapshots_controller.rb:11:in `create'
  actionpack (4.0.2) lib/action_controller/metal/implicit_render.rb:4:in `send_action'
  actionpack (4.0.2) lib/abstract_controller/base.rb:189:in `process_action'
  actionpack (4.0.2) lib/action_controller/metal/rendering.rb:10:in `process_action'
  actionpack (4.0.2) lib/abstract_controller/callbacks.rb:18:in `block in process_action'
  activesupport (4.0.2) lib/active_support/callbacks.rb:393:in `_run__1667715092399278678__process_action__callbacks'
  activesupport (4.0.2) lib/active_support/callbacks.rb:80:in `run_callbacks'
  actionpack (4.0.2) lib/abstract_controller/callbacks.rb:17:in `process_action'
  actionpack (4.0.2) lib/action_controller/metal/rescue.rb:29:in `process_action'
  actionpack (4.0.2) lib/action_controller/metal/instrumentation.rb:31:in `block in process_action'
  activesupport (4.0.2) lib/active_support/notifications.rb:159:in `block in instrument'
  activesupport (4.0.2) lib/active_support/notifications/instrumenter.rb:20:in `instrument'
  activesupport (4.0.2) lib/active_support/notifications.rb:159:in `instrument'
  actionpack (4.0.2) lib/action_controller/metal/instrumentation.rb:30:in `process_action'                                                                               [20/9017]
  actionpack (4.0.2) lib/action_controller/metal/params_wrapper.rb:245:in `process_action'
  activerecord (4.0.2) lib/active_record/railties/controller_runtime.rb:18:in `process_action'
  actionpack (4.0.2) lib/abstract_controller/base.rb:136:in `process'
  actionpack (4.0.2) lib/abstract_controller/rendering.rb:44:in `process'
  actionpack (4.0.2) lib/action_controller/metal.rb:195:in `dispatch'
  actionpack (4.0.2) lib/action_controller/metal/rack_delegation.rb:13:in `dispatch'
  actionpack (4.0.2) lib/action_controller/metal.rb:231:in `block in action'
  actionpack (4.0.2) lib/action_dispatch/routing/route_set.rb:80:in `dispatch'
  actionpack (4.0.2) lib/action_dispatch/routing/route_set.rb:48:in `call'
  actionpack (4.0.2) lib/action_dispatch/journey/router.rb:71:in `block in call'
  actionpack (4.0.2) lib/action_dispatch/journey/router.rb:59:in `call'
  actionpack (4.0.2) lib/action_dispatch/routing/route_set.rb:680:in `call'
  rack (1.5.2) lib/rack/etag.rb:23:in `call'
  rack (1.5.2) lib/rack/conditionalget.rb:35:in `call'
  rack (1.5.2) lib/rack/head.rb:11:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/params_parser.rb:27:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/flash.rb:241:in `call'
  rack (1.5.2) lib/rack/session/abstract/id.rb:225:in `context'
  rack (1.5.2) lib/rack/session/abstract/id.rb:220:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/cookies.rb:486:in `call'
  activerecord (4.0.2) lib/active_record/query_cache.rb:36:in `call'
  activerecord (4.0.2) lib/active_record/connection_adapters/abstract/connection_pool.rb:626:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/callbacks.rb:29:in `block in call'
  activesupport (4.0.2) lib/active_support/callbacks.rb:373:in `_run__4319389214609866823__call__callbacks'
  activesupport (4.0.2) lib/active_support/callbacks.rb:80:in `run_callbacks'
  actionpack (4.0.2) lib/action_dispatch/middleware/callbacks.rb:27:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/reloader.rb:64:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/remote_ip.rb:76:in `call'
  better_errors (1.1.0) lib/better_errors/middleware.rb:84:in `protected_app_call'
  better_errors (1.1.0) lib/better_errors/middleware.rb:79:in `better_errors_call'
  better_errors (1.1.0) lib/better_errors/middleware.rb:56:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/debug_exceptions.rb:17:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/show_exceptions.rb:30:in `call'
  railties (4.0.2) lib/rails/rack/logger.rb:38:in `call_app'
  railties (4.0.2) lib/rails/rack/logger.rb:20:in `block in call'
  activesupport (4.0.2) lib/active_support/tagged_logging.rb:67:in `block in tagged'
  activesupport (4.0.2) lib/active_support/tagged_logging.rb:25:in `tagged'
  activesupport (4.0.2) lib/active_support/tagged_logging.rb:67:in `tagged'
  railties (4.0.2) lib/rails/rack/logger.rb:20:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/request_id.rb:21:in `call'
  rack (1.5.2) lib/rack/methodoverride.rb:21:in `call'
  rack (1.5.2) lib/rack/runtime.rb:17:in `call'
  activesupport (4.0.2) lib/active_support/cache/strategy/local_cache.rb:83:in `call'
  rack (1.5.2) lib/rack/lock.rb:17:in `call'
  actionpack (4.0.2) lib/action_dispatch/middleware/static.rb:64:in `call'
  rack (1.5.2) lib/rack/sendfile.rb:112:in `call'
  railties (4.0.2) lib/rails/engine.rb:511:in `call'
  railties (4.0.2) lib/rails/application.rb:97:in `call'
  rack (1.5.2) lib/rack/lock.rb:17:in `call'
  rack (1.5.2) lib/rack/content_length.rb:14:in `call'
  rack (1.5.2) lib/rack/handler/webrick.rb:60:in `service'
  /Users/joelencioni/.rbenv/versions/2.1.0/lib/ruby/2.1.0/webrick/httpserver.rb:138:in `service'
  /Users/joelencioni/.rbenv/versions/2.1.0/lib/ruby/2.1.0/webrick/httpserver.rb:94:in `run'
  /Users/joelencioni/.rbenv/versions/2.1.0/lib/ruby/2.1.0/webrick/server.rb:295:in `block in start_thread'

When taking snapshots that encounter problems, surface the problems to the user

A host of problems may happen when taking a snapshot. Was there a problem with the SSL certificate? Did the server timeout? These types of problems should be apparent to the person using Diffux to create snapshots so they can understand what happened and react accordingly.

Disable jQuery animations

Snapshot may end up being taken at slightly different points in the animation, causing a visual difference when there really is no difference. This has the potential to cause a lot of noise.

Since many sites rely on jQuery for animations, we can reduce the amount of noise caused by this problem by disabling jQuery animations when the page loads, if jQuery is available. We can do this by setting $.fx.off to true. More info:

http://api.jquery.com/jQuery.fx.off/

There may be other things that we can do to reduce the noise caused by animations, but this seems like a pretty good start.

Need logo and favicon

Images at wider viewport widths can be too small to interpret

We could fix this by either just showing larger images, by allowing a click to zoom sort of thing, or both.

Tests don't pass

Allow snapshots to be triggered from external signal

We want to have snapshots triggered every time we deploy a new version of our app. For this to work, diffux needs to listen for a signal from an external source that triggers snapshots.

Ideally, these snapshots would receive some associated metadata, such as the name of the deploy, and perhaps a copy of the release notes.

Not resilient against JS errors

I recently ran a snapshot on a page that had a JavaScript error because an object that was trying to be accessed was not defined. This caused the following output from my Sidekiq worker:

2014-02-07T00:16:13Z 95193 TID-oxt5613ng WARN: {"retry"=>true, "queue"=>"default", "class"=>"SnapshotWorker", "args"=>[234], "jid"=>"9fbbafc09342a5a2b1ca9560", "enqueued_at"=>1391731933.306431, "error_message"=>"ReferenceError: Can't find variable: olark\n", "error_class"=>"RuntimeError", "failed_at"=>"2014-02-07T00:12:18Z", "retry_count"=>3, "retried_at"=>2014-02-07 00:16:13 UTC}
2014-02-07T00:16:13Z 95193 TID-oxt5613ng WARN: ReferenceError: Can't find variable: olark

Add change animation

We shouldn't worry about this for a long while, but it would be really cool to be able to generate an animation of how each URL/Viewport has changed over time. Just cycle through the accepted images with > 0% diffs and put together an animation. It would be really cool to see how things evolve.

Auto-accept can make snapshots be both rejected and accepted

If a snapshot has once been auto-accepted and you later reject it, the snapshot will be saved with both an accepted_at and rejected_at date. When rejecting/accepting manually, we always nil the other state.

Add re-take snapshot option when viewing snapshot

Sometimes you may be looking at a snapshot and thinking that some weird glitch happened that time that would be fixed by simply running the snapshot again. There should be an easy way to re-take the snapshot right from that page.

Failed snapshots should be marked

If a snapshot cannot complete for whatever reason, it should be marked with a red "X" icon instead of remaining in the pending state. We can probably define failure as the Sidekiq job no longer existing in the queue (though I'm not sure that is the best way to check for this condition)

Move over to rspec

I (and other contributors) are more familiar with this than the internal rails test framework.

Add way to easily share diffs

If you are running Diffux in a protected environment (e.g. behind a firewall or just on localhost) but you want to share a diff with someone who does not have the same access as you, there should be an easy way to do that.

Possibly interface with a service like Imgur or something.

Use better viewport heights

We are currently setting the viewport height to be viewport width * 2. For narrow viewport widths this actually makes some sense. For example, phones are often held in portrait orientation where the height is somewhere within the zone of twice the width. However, for wider viewport widths, this is fairly uncommon--laptop screens and desktop monitors are usually in landscape orientation.

Add keyboard shortcuts

We want diffux to be fast and easy to use for power users. This means keyboard shortcuts.

For this use case, it probably makes sense to model things after Gmail or Gerrit. j/k to move up/down lists, [/] to move between items, u to move up a level. ? to bring up keyboard shortcuts list.

Doesn't work well with lazily loaded content

On pages that lazily AJAX in content after the page has loaded, the snapshot will sometimes be taken before the content has a chance to come back, and sometimes after the content has been added to the page (see image below for an example).

We need to figure out a good way for our snapshot script to know when a page is actually ready for a snapshot.

Add email field to sweeps, and send email when sweep is done

This will be awesome for our deploy process. If we send the email with the subject field set to "Re: {sweep.title}", there is a good chance the sweep will be put in the same email thread as the deploy email (if such a thing was sent out).

Removed Cloudinary, here's how to migrate and keep all previous snapshots

Ping @lencioni @kbsail @hellobelda

I'm filing this issue to inform you that I've just made some changes that, if not performed in the right order, will delete all the previous snapshots you've made.

This commit moves us from saving snapshot images on Cloudinary to a local solution:
6dade5f

The commit after that introduces a tool to manually migrate all Cloudinary images:
cee43a3

The third commit removes all Cloudinary fields altogether:
a8038c1

If you haven't already installed ImageMagick, you should do so first:

brew install imagemagick

If you want to keep all your previous snapshots, you should do this:

git fetch
git rebase
git checkout cee43a3
bundle exec rake db:migrate db:test:prepare
bundle exec rails runner "ManualMigrations.move_snapshot_images\!"

That last command might be fast or slow depending on how many images you have.

Double check in your browser so that you are still seeing snapshot images.

Lastly, we can go back to the master branch and run the last migration:

git checkout -
bundle exec rake db:migrate db:test:prepare

That should get rid of the last bits of Cloudinary.

Sweep should be part of Snapshot breadcrumb trail

For snapshots that belong to a sweep, that sweep should be part of the breadcrumb trail when viewing the snapshot. This will improve the navigation of the app.

Unify error messaging for forms

We've got a couple of similar HAML blocks of displaying errors on a form that we should unify into a shared partial/helper.

See
app/views/sweeps/_form.html.haml

Extract services from snapshot model

Right now, snapshotting and image diffing happens in a before_save filter on the model. To make this more robust, I want to extract these into two services. The good thing about that is that it should be easy to refactor/try other approaches to these services in the future.

Auto-generate titles for URLs

Since we are fetching the page anyway, we can look at information in the document to generate good titles for our URLs automatically. Fall back to URL for title.

PhantomJS support HTML5 Video?

Hello -

I included a 5 second timeout with the PhantomJS rendering - Wanted to see if it would work to then build up later to only taking the screenshot until after the AJAX / JavaScript has fully loaded.

The timeout allowed the additional AJAX / JavaScript to load (I used the causes website), however, it did not render the video.

I looked into it and it says HTML5 video / Flash is not supported: https://github.com/ariya/phantomjs/wiki/Supported-Web-Standards

PhantomJS say they will not be supporting audio/video in the near future - Is this correct? Not really important as it is a layout tool and at least the space where the video is imbedded is represented by the annoying "Video not supported" message.

Swap before and after image placement

The before image should appear on the left and the after image should appear on the right. This matches our left-to-right reading style and also what we often see in code review tools.

Indicate baseline, needs review, reviewed snapshots on UrlsController#show

Looking at a sea of snapshots, it is difficult to tell where we are at in the review process. There should be an indication of which snapshot is the baseline, which need to be reviewed, and which have already been reviewed when viewing the thumbnails.