Skip to content

Move showcase source data into a JSON file - #12626

Closed
hramos wants to merge 4 commits into
react:masterfrom
hramos:showcase-tests
Closed

hramos wants to merge 4 commits into
react:masterfrom
hramos:showcase-tests

Conversation

@hramos

@hramos hramos commented Feb 28, 2017

Copy link
Copy Markdown
Contributor

The showcase is displayed in the homepage as well as in a standalone showcase page. The source data for these two pages is embedded in each page, resulting in repetition of data across both.

This PR splits out all of the showcase data to its own file, which is then loaded into the Metadata component during website generation.

Test Plan

cd website && npm start, then verified index and showcase loaded successfully
cd website && node server/generate.js, confirmed script runs successfully

@facebook-github-bot facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Feb 28, 2017

@ericvicenti ericvicenti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looking great!

Comment thread website/showcase.json
"linkPlayStore": "https://play.google.com/store/apps/details?id=com.cbssports.fantasy.franchisefootball2015",
"infoLink": "http://www.cbssports.com/fantasy/football/games/franchise/2015",
"infoTitle": "Award winning Fantasy Football league manager",
"pinned": false

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

having pinned=false seems verbose. You could just leave it off unless it is actually pinned

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That was my original approach, but I could not find a clean way to filter for unpinned entries in https://github.com/facebook/react-native/pull/12626/files#diff-deb30c36b81d11c5a077a65d4e2d9baaR60 in that case.

Variations I tried that failed to only return objects missing the pinned key (or have pinned: false):

const featuredApps = showcaseApps.filter(app => {
  return !app.hasOwnProperty('pinned') || !app.pinned;
}).sort(function(a, b) {
  return a.name.localeCompare(b.name);
});

Comment thread website/server/convert.js
});

// load showcase apps into metadata
var showcaseApps = JSON.parse(fs.readFileSync(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

in other codebases we would want flow types, but nevermind here 😄

@facebook-github-bot facebook-github-bot added GH Review: accepted Import Started This pull request has been imported. This does not imply the PR has been approved. and removed GH Review: review-needed labels Mar 1, 2017
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

2 similar comments
@facebook-github-bot

Copy link
Copy Markdown
Contributor

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot

Copy link
Copy Markdown
Contributor

@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@hramos
hramos deleted the showcase-tests branch March 6, 2017 17:45
hramos added a commit that referenced this pull request Mar 9, 2017
Summary:
The showcase is displayed in the homepage as well as in a standalone showcase page. The source data for these two pages is embedded in each page, resulting in repetition of data across both.

This PR splits out all of the showcase data to its own file, which is then loaded into the Metadata component during website generation.

`cd website && npm start`, then verified index and showcase loaded successfully
`cd website && node server/generate.js`, confirmed script runs successfully
Closes #12626

Differential Revision: D4632036

Pulled By: hramos

fbshipit-source-id: 2d1ad890e78e457205179e36c3ef04ffec354ad9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Import Started This pull request has been imported. This does not imply the PR has been approved.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants