-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
Move showcase source data into a JSON file #12626
Conversation
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.
Looking great!
"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 |
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.
having pinned=false seems verbose. You could just leave it off unless it is actually pinned
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.
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);
});
@@ -179,6 +180,13 @@ function execute(options) { | |||
metadatas.config[key] = process.env[key]; | |||
}); | |||
|
|||
// load showcase apps into metadata | |||
var showcaseApps = JSON.parse(fs.readFileSync( |
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.
in other codebases we would want flow types, but nevermind here 😄
@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
2 similar comments
@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
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
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 successfullycd website && node server/generate.js
, confirmed script runs successfully