Skip to content
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.

Sign up for GitHub

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

docs(Ads): fix rendering in SSR #3218

Merged
merged 4 commits into from
Oct 20, 2018
Merged

docs(Ads): fix rendering in SSR #3218

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
91026cf
fix(Ads): fix rendering in SSR
layershifter Oct 15, 2018
b3b643e
fix(Document): add version to style
layershifter Oct 15, 2018
b844e54
fix(build): fix regexp for Windows
Oct 18, 2018
6bebdea
Merge branch 'master' into fix/ads
layershifter Oct 20, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions docs/src/components/CarbonAd/CarbonAd.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,23 +1,31 @@
import _ from 'lodash'
import React, { Component } from 'react'

import { isBrowser } from 'src/lib'

const style = {
padding: '0.5rem',
background: '#222',
boxShadow: '0 0 2rem black',
}

let isLoading = true
let script

const adExist = () => !!document.querySelector('#docs-carbonads #carbonads')

const script = document.createElement('script')
script.async = true
script.id = '_carbonads_js'
script.type = 'text/javascript'
script.src = '//cdn.carbonads.com/carbon.js?serve=CK7DT23J&placement=reactsemanticuicom'
script.onload = () => {
isLoading = false
// Heads up!
// We render docs with React-Static which performs SSR rendering.
if (isBrowser()) {
script = document.createElement('script')

script.async = true
script.id = '_carbonads_js'
script.type = 'text/javascript'
script.src = '//cdn.carbonads.com/carbon.js?serve=CK7DT23J&placement=reactsemanticuicom'
script.onload = () => {
isLoading = false
}
}

const waitForLoad = () => {
Expand Down
2 changes: 1 addition & 1 deletion docs/src/components/Document.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const Document = ({ Body, children, Head, Html, siteData: { dev, versions } }) =
<meta name='viewport' content='width=device-width, initial-scale=1, shrink-to-fit=no' />

<link rel='shortcut icon' type='image/x-icon' href='/logo.png' />
<link rel='stylesheet' href='/style.css' />
<link rel='stylesheet' href={`/style.css?${versions.suir}`} />
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We added changes to styles in #3215, but they can be cached because we don't manage these styles by Webpack (i.e. content hash or other hash).

<link
rel='stylesheet'
href={`https://cdn.jsdelivr.net/npm/semantic-ui-css@${versions.sui}/semantic.min.css`}
Expand Down
2 changes: 1 addition & 1 deletion gulp/tasks/docs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ task(
// ----------------------------------------

task('build:docs:static:build', (cb) => {
build().then(cb)
build({ staging: !!process.env.STAGING }).then(cb)
})

task('build:docs:static:reload', (cb) => {
Expand Down
2 changes: 1 addition & 1 deletion gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
require('@babel/register')({
ignore: [/node_modules/, /docs\/dist/],
ignore: [/node_modules/, /docs[\\/]dist/],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also added a fix for build on Windows

})

const { task, series, parallel } = require('gulp')
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@
"scripts": {
"build": "cross-env NODE_ENV=production gulp build",
"build:changelog": "github_changelog_generator --no-issues --no-unreleased --release-branch master --since-tag $(git describe --abbrev=0 --tags $(git rev-parse HEAD~300))",
"build:docs": "gulp --series build:docs",
"build:docs": "cross-env NODE_ENV=production gulp build:docs",
"build:docs:staging": "cross-env STAGING=true yarn build:docs && yarn serve docs/dist",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this for local development? If so, should it be NODE_ENV=development? If not, why not NODE_ENV=staging?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This for testing production build of docs locally

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NODE_ENV=test?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main idea that build will be production, so you will have the same result as will be on react.semantic-ui.com, i.e. manual predeployment check.

The current production build cannot be tested locally because it has hardcoded links to react.semantic-ui.com.

So we can't use there NODE_ENV variable, however we can remove this change at all. I thought that it would be useful for you.

"build:dist": "gulp --series build:dist",
"ci": "yarn tsd:lint && yarn tsd:test && yarn lint && yarn test",
"predeploy:docs": "cross-env NODE_ENV=production yarn build:docs && gulp build:docs:cname",
Expand Down