350 open graph react #360

Merged
bones7242 merged 30 commits from 350-open-graph-react into master 2018-02-24 17:55:00 +01:00
6 changed files with 88 additions and 188 deletions
Showing only changes of commit 60b2b5f203 - Show all commits

View file

@ -30,17 +30,6 @@ sequelize
logger.error('Sequelize was unable to connect to the database:', err);
});
// // add each model to the db object
// fs
// .readdirSync(__dirname)
// .filter(file => {
// return (file.indexOf('.') !== 0 && file !== basename && file.slice(-3) === '.js');
// })
// .forEach(file => {
// const model = sequelize['import'](path.join(__dirname, file));
// db[model.name] = model;
// });
// manually add each model to the db
const Certificate = require('./certificate.js');
const Channel = require('./channel.js');

View file

@ -6,12 +6,13 @@
"scripts": {
"test": "mocha --recursive",
"test-all": "mocha --recursive",
"start": "nodemon server.js",
"start": "server.js",
"lint": "eslint .",
"fix": "eslint . --fix",
"precommit": "eslint .",
"babel": "babel",
"webpack": "webpack"
"build-dev": "webpack",
"build-production": "webpack"
},
"repository": {
"type": "git",
@ -89,6 +90,7 @@
"nodemon": "^1.15.1",
"redux-devtools": "^3.4.1",
"regenerator-transform": "^0.12.3",
"webpack": "^3.10.0"
"webpack": "^3.10.0",
"webpack-merge": "^4.1.2"
}
}

View file

@ -24,10 +24,8 @@ const reduxMiddleware = window.__REDUX_DEVTOOLS_EXTENSION__ ? compose(middleware
neb-b commented 2018-02-23 20:57:29 +01:00 (Migrated from github.com)
Review

Probably don't need these console.logs here.

Probably don't need these `console.log`s here.
neb-b commented 2018-02-23 20:57:29 +01:00 (Migrated from github.com)
Review

Probably don't need these console.logs here.

Probably don't need these `console.log`s here.
// create teh store
let store;
if (preloadedState) {
console.log('initial load: preloaded state found');
neb-b commented 2018-02-23 20:57:29 +01:00 (Migrated from github.com)
Review

Probably don't need these console.logs here.

Probably don't need these `console.log`s here.
store = createStore(Reducer, preloadedState, reduxMiddleware);
} else {
console.log('initial load: no preloaded state found');
neb-b commented 2018-02-23 20:57:29 +01:00 (Migrated from github.com)
Review

Probably don't need these console.logs here.

Probably don't need these `console.log`s here.
store = createStore(Reducer, reduxMiddleware);
}

neb-b commented 2018-02-23 20:57:29 +01:00 (Migrated from github.com)
Review

Probably don't need these console.logs here.

Probably don't need these `console.log`s here.
neb-b commented 2018-02-23 20:57:29 +01:00 (Migrated from github.com)
Review

Probably don't need these console.logs here.

Probably don't need these `console.log`s here.

View file

@ -1,21 +0,0 @@
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
import { connect } from 'react-redux';
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
import View from './view';
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
const mapStateToProps = ({ show }) => {
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
// select request info
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
const requestId = show.request.id;
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
// select asset info
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
let asset;
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
const request = show.requestList[requestId] || null;
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
const assetList = show.assetList;
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
if (request && assetList) {
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
const assetKey = request.key; // note: just store this in the request
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
asset = assetList[assetKey] || null;
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
};
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
// return props
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
return {
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
asset,
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
};
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
};
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.
export default connect(mapStateToProps, null)(View);
neb-b commented 2018-02-23 21:40:39 +01:00 (Migrated from github.com)
Review

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the show reducer here, which I don't really think needs to exist (in the future).

Then you wouldn't have to worry about the show reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)

I'm probably missing something here, but wherever this is rendered, don't you already have the asset? You could just directly pass it in (or just pass some identifier to easily find it in state). Then you wouldn't need the `show` reducer here, which I don't really think needs to exist (in the future). Then you wouldn't have to worry about the `show` reducer being populated and could use these tags for other pages (not sure if you would ever need to do that though)
bones7242 commented 2018-02-24 00:47:01 +01:00 (Migrated from github.com)
Review

ah, yes. This component should actually be deleted. I made it before moving everything into the <SEO /> component.

ah, yes. This component should actually be deleted. I made it before moving everything into the `<SEO />` component.

View file

@ -1,83 +0,0 @@
import React from 'react';
import Helmet from 'react-helmet';
const { site: { title, host }, claim: { defaultThumbnail, defaultDescription } } = require('../../../config/speechConfig.js');
const determineOgThumbnailContentType = (thumbnail) => {
if (thumbnail) {
const fileExt = thumbnail.substring(thumbnail.lastIndexOf('.'));
switch (fileExt) {
case 'jpeg':
case 'jpg':
return 'image/jpeg';
case 'png':
return 'image/png';
case 'gif':
return 'image/gif';
case 'mp4':
return 'video/mp4';
default:
return 'image/jpeg';
}
}
return '';
};
class OpenGraphTags extends React.Component {
render () {
const { claimData } = this.props.asset;
const { contentType } = claimData;
const embedUrl = `${host}/${claimData.claimId}/${claimData.name}`;
const showUrl = `${host}/${claimData.claimId}/${claimData.name}`;
const source = `${host}/${claimData.claimId}/${claimData.name}.${claimData.fileExt}`;
const ogTitle = claimData.title || claimData.name;
const ogDescription = claimData.description || defaultDescription;
const ogThumbnailContentType = determineOgThumbnailContentType(claimData.thumbnail);
const ogThumbnail = claimData.thumbnail || defaultThumbnail;
return (
<div>
<Helmet>
{/* basic open graph tags */}
<meta property='og:title' content={ogTitle} />
<meta property='og:url' content={showUrl} />
<meta property='og:site_name' content={title} />
<meta property='og:description' content={ogDescription} />
<meta property='og:image:width' content='600' />
<meta property='og:image:height' content='315' />
{/* basic twitter tags */}
<meta name='twitter:site' content='@spee_ch' />
</Helmet>
{ contentType === 'video/mp4' || contentType === 'video/webm' ? (
<Helmet>
{/* video open graph tags */}
<meta property='og:video' content={source} />
<meta property='og:video:secure_url' content={source} />
<meta property='og:video:type' content={contentType} />
<meta property='og:image' content={ogThumbnail} />
<meta property='og:image:type' content={ogThumbnailContentType} />
<meta property='og:type' content='video' />
{/* video twitter tags */}
<meta name='twitter:card' content='player' />
<meta name='twitter:player' content={embedUrl} />
<meta name='twitter:player:width' content='600' />
<meta name='twitter:text:player_width' content='600' />
<meta name='twitter:player:height' content='337' />
<meta name='twitter:player:stream' content={source} />
<meta name='twitter:player:stream:content_type' content={contentType} />
</Helmet>
) : (
<Helmet>
{/* image open graph tags */}
<meta property='og:image' content={source} />
<meta property='og:image:type' content={contentType} />
<meta property='og:type' content='article' />
{/* image twitter tags */}
<meta name='twitter:card' content='summary_large_image' />
</Helmet>
)}
</div>
);
}
};
export default OpenGraphTags;

View file

@ -1,41 +1,10 @@
const Path = require('path');
neb-b commented 2018-02-23 21:17:39 +01:00 (Migrated from github.com)
Review

I would recommend creating separate webpack configs for dev/prod. Before we did our big restructure changes the apps webpack setup had three files:

A base config: contains the generic stuff like babel, entry/output points, css loader, etc.
A dev config which extends the base config: contains stuff like watch: true (not needed in prod) and source maps
A prod config which extends the base config: contains stuff like minification (that's the main thing)

webpack-merge is a great tool for this
https://github.com/survivejs/webpack-merge

I would recommend creating separate webpack configs for dev/prod. Before we did our big restructure changes the apps webpack setup had three files: A base config: contains the generic stuff like babel, entry/output points, css loader, etc. A dev config which extends the base config: contains stuff like `watch: true` (not needed in prod) and source maps A prod config which extends the base config: contains stuff like minification (that's the main thing) `webpack-merge` is a great tool for this https://github.com/survivejs/webpack-merge
const nodeExternals = require('webpack-node-externals');
const REACT_ROOT = Path.resolve(__dirname, 'react/');
const merge = require('webpack-merge');
const TARGET = process.env.npm_lifecycle_event;
console.log('REACT_ROOT:', REACT_ROOT);
module.exports = [
{
target: 'web',
entry : ['babel-polyfill', 'whatwg-fetch', './react/client.js'],
output: {
path : Path.join(__dirname, 'public/bundle/'),
publicPath: 'public/bundle/',
filename : 'bundle.js',
},
watch : true,
module: {
loaders: [
{
test : /.jsx?$/,
loader : 'babel-loader',
exclude: /node_modules/,
query : {
presets: ['es2015', 'react', 'stage-2'],
},
},
],
},
resolve: {
modules: [
REACT_ROOT,
'node_modules',
__dirname,
],
extensions: ['.js', '.jsx', '.scss'],
},
},
{
const serverBaseConfig = {
target: 'node',
node : {
__dirname: false,
@ -47,7 +16,6 @@ module.exports = [
publicPath: '/',
filename : 'server.js',
},
watch : true,
module: {
rules: [
{
@ -72,5 +40,52 @@ module.exports = [
],
extensions: ['.js', '.json', '.jsx', '.css'],
},
};
const clientBaseConfig = {
target: 'web',
entry : ['babel-polyfill', 'whatwg-fetch', './react/client.js'],
output: {
path : Path.join(__dirname, 'public/bundle/'),
publicPath: 'public/bundle/',
filename : 'bundle.js',
},
];
module: {
loaders: [
{
test : /.jsx?$/,
loader : 'babel-loader',
exclude: /node_modules/,
query : {
presets: ['es2015', 'react', 'stage-2'],
},
},
],
},
resolve: {
modules: [
REACT_ROOT,
'node_modules',
__dirname,
],
extensions: ['.js', '.jsx', '.scss'],
},
};
if (TARGET === 'build-dev') {
module.exports = [
merge(serverBaseConfig, {
watch: true,
}),
merge(clientBaseConfig, {
watch: true,
}),
];
};
if (TARGET === 'build-production') {
module.exports = [
merge(clientBaseConfig, {}),
merge(serverBaseConfig, {}),
];
};