-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adds collection items #22
Conversation
@@ -19,6 +19,8 @@ | |||
"dist:copy-static": "cp -r src/static dist/static", | |||
"dist-static:clean": "rm -rf dist_static && mkdir dist_static", | |||
"dist-static:copy-static": "cp -r src/static dist_static/static", | |||
"lint": "eslint ./src/", |
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.
We can now lint things :)
@@ -0,0 +1,41 @@ | |||
import React from 'react' |
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.
Mixes Route and Redux. Not sure if this is best way to do it. But otherwise I would be creating a route that just has a container than then has the same stuff. Extra layer, didn't seem to be a good reason for it.
@nickmask Ready for code review :) You will need to do an |
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.
Nice work, really clean code.
Merge approved with minor changes.
const DeepZoom = (props) => { | ||
return ( | ||
<div> | ||
<iframe width="640" height="360" src={props.url} frameBorder="0" allowFullScreen /> |
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.
- Need to change width to 100%. Currently cuts off options with the fixed width.
- Full screen mode currently doesn't work. Looks like an issue with GigaPan, so unsure what we can do here. Full screen mode does work on the 3D models.
const Model3d = (props) => { | ||
return ( | ||
<div> | ||
<iframe width="640" height="480" src={props.url} frameBorder="0" allowFullScreen /> |
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.
Need to change width to 100%. Height is also a bit much for the iphone 4, but we can deal with that when we style.
Closes: