This repository has been archived by the owner on Jun 11, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 16
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
We're about to make the diff review experience better. To make future enhancements easier, we're moving over the UI to React. This will allow us to work inside an environment that we're all (Galooshi members) familiar with. I've been pushing back on introducing unnecessary complexity into this project. We've had a fruitful discussion in #61 about possible ways to move forward, and what I'm presenting here is basically a middle ground between what @lencioni is suggesting and what I've been advocating for. I'm making the page depend on three externally hosted scripts: - React - so that we can write nice UIs - ReactDOM - so that we can hook up those UIs to the DOM - babel - so that we can use jsx, string templates, arrow functions and such. The in-browser transpilation done by babel isn't the fastest, but I don't see this as a bottleneck at this point. If the application continues to grow, it might be worth introducing a build step to the process. But at this point I don't think it's necessary. When I was porting over the image_slug method, I had some issue reimplementing it in javascript. So I decided to delegate to window.btoa() instead, which will simply base64 encode the string. This fixes a potential naming collision issue, but at the cost of making the URL less readable. Simplicity and robustness won this time, but there's a chance we'll move over to more readable slugs at some later point in time. This change shouldn't result in any differences in the UI (except for the already-mentioned slug change). Future commits will focus on improving it.
- Loading branch information
Showing
5 changed files
with
117 additions
and
57 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
require 'json' | ||
require 'happo/utils' | ||
require 'happo/uploader' | ||
require 'happo/version' | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
const PropTypes = React.PropTypes; | ||
|
||
const imageObjectStructure = { | ||
description: PropTypes.string.isRequired, | ||
viewport: PropTypes.string.isRequired, | ||
url: PropTypes.string.isRequired, | ||
}; | ||
|
||
function imageSlug(image) { | ||
// TODO can we just use btoa() here? | ||
return btoa(image.description + image.viewport); | ||
} | ||
|
||
function renderImage(image) { | ||
return ( | ||
<div> | ||
<h3 id={imageSlug(image)}> | ||
<a className='anchored' href={`#${imageSlug(image)}`}> | ||
{image.description} | ||
{' @ '} | ||
{image.viewport} | ||
</a> | ||
</h3> | ||
<img src={image.url} /> | ||
</div> | ||
); | ||
} | ||
|
||
function renderDiffImages(diffImages) { | ||
if (!diffImages.length) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<div> | ||
<h2 id='diffs'> | ||
<a className='anchored' href='#diffs'> | ||
Diffs ({ diffImages.length }) | ||
</a> | ||
</h2> | ||
|
||
{diffImages.map(renderImage)} | ||
</div> | ||
); | ||
} | ||
|
||
function renderNewImages(newImages) { | ||
if (!newImages.length) { | ||
return null; | ||
} | ||
|
||
return ( | ||
<div> | ||
<h2 id='new'> | ||
<a className='anchored' href='#new'> | ||
New examples ({ newImages.length }) | ||
</a> | ||
</h2> | ||
|
||
{newImages.map(renderImage)} | ||
</div> | ||
); | ||
} | ||
|
||
window.HappoDiffs = React.createClass({ | ||
propTypes: { | ||
pageTitle: PropTypes.string.isRequired, | ||
diffImages: PropTypes.arrayOf(imageObjectStructure).isRequired, | ||
newImages: PropTypes.arrayOf(imageObjectStructure).isRequired, | ||
generatedAt: PropTypes.string.isRequired, | ||
}, | ||
|
||
render() { | ||
return ( | ||
<div> | ||
<header className='header'> | ||
<h1 className='header_title'> | ||
{this.props.pageTitle} | ||
</h1> | ||
<div className='header__time'> | ||
Generated: {this.props.generatedAt} | ||
</div> | ||
</header> | ||
|
||
<main className='main'> | ||
{renderDiffImages(this.props.diffImages)} | ||
{renderNewImages(this.props.newImages)} | ||
</main> | ||
</div> | ||
); | ||
} | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,52 +10,27 @@ | |
<style> | ||
<%= Happo::Utils.css_styles %> | ||
</style> | ||
</head> | ||
|
||
<body> | ||
<header class="header"> | ||
<h1 class="header_title"> | ||
<%= Happo::Utils.page_title(diff_images, new_images) %> | ||
</h1> | ||
<div class="header__time">Generated: <%= Time.now %></div> | ||
</header> | ||
|
||
<main class="main"> | ||
<% if diff_images.count > 0%> | ||
<h2 id="diffs"> | ||
<a class="anchored" href="#diffs"> | ||
Diffs (<%= diff_images.count %>) | ||
</a> | ||
</h2> | ||
|
||
<% diff_images.each do |image| %> | ||
<h3 id="<%= Happo::Utils.image_slug(image) %>"> | ||
<a class="anchored" href="#<%= Happo::Utils.image_slug(image) %>"> | ||
<%= Rack::Utils.escape_html(image[:description]) %> @ <%= image[:viewport] %> | ||
</a> | ||
</h3> | ||
<script src="https://npmcdn.com/[email protected]/dist/react.min.js"></script> | ||
<script src="https://npmcdn.com/[email protected]/dist/react-dom.min.js"></script> | ||
<script src="https://npmcdn.com/[email protected]/browser.min.js"></script> | ||
|
||
<img src="<%= image[:url] %>"> | ||
<% end %> | ||
<% end %> | ||
<% if new_images.count > 0%> | ||
<h2 id="new"> | ||
<a class="anchored" href="#new"> | ||
New examples (<%= new_images.count %>) | ||
</a> | ||
</h2> | ||
|
||
<% new_images.each do |image| %> | ||
<h3 id="<%= Happo::Utils.image_slug(image) %>"> | ||
<a class="anchored" href="#<%= Happo::Utils.image_slug(image) %>"> | ||
<%= Rack::Utils.escape_html(image[:description]) %> @ <%= image[:viewport] %> | ||
</a> | ||
</h3> | ||
<script type="text/babel"> | ||
<%= Happo::Utils.jsx_code %> | ||
</script> | ||
</head> | ||
|
||
<img src="<%= image[:url] %>"> | ||
<% end %> | ||
<% end %> | ||
</main> | ||
<body> | ||
<script type="text/babel"> | ||
ReactDOM.render( | ||
<HappoDiffs | ||
newImages={<%= new_images.to_json %>} | ||
diffImages={<%= diff_images.to_json %>} | ||
pageTitle={<%= Happo::Utils.page_title(diff_images, new_images).to_json %>} | ||
generatedAt={<%= Time.now.to_s.to_json %>} | ||
/>, | ||
document.body | ||
); | ||
</script> | ||
</body> | ||
</html> |