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.

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

site-specific components in component lists #510

Merged
merged 6 commits into from Jul 8, 2016

Conversation

nelsonpecora
Copy link
Contributor

fixes #492

allows you to specify that certain components in lists should be included/excluded only on certain sites

function availableOnCurrentSite(slug, logic) {
var tokens = logic.split(',').map((str) => str.trim()), // trim any extra whitespace
// list of site slugs to include
include = _.reject(tokens, (token) => _.includes(token, 'not:')),
Copy link
Contributor

Choose a reason for hiding this comment

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

could you call these "sitesToInclude" "sitesToExclude"? Self-documenting variable names!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fiiiiiiiine

@byronhulcher
Copy link
Contributor

👍 with some minor comments about comments

- article (site1, site2) # allow on site1 and site2 ONLY
- paragraph # allow on all sites
- image (not:site1) # allow on all sites EXCEPT site1
- link (site1, site2, site3, not:site2, not:site3) # only allow on site1
Copy link

Choose a reason for hiding this comment

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

Hmm, do site-specific options belong in the schema? Seems like schema should be only tied to the component and generic to any site.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as a less-used +1, I think this is fine, especially since it's only for implementation-specific components (adding specific sites to a list in a component on npm would be dumb)

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 79.545% when pulling cf7d321 on site-specific-lists into 08780d3 on master.

@nelsonpecora
Copy link
Contributor Author

👍 from byron

@nelsonpecora nelsonpecora merged commit 6b49654 into master Jul 8, 2016
@nelsonpecora nelsonpecora deleted the site-specific-lists branch July 8, 2016 21:12
@byronhulcher
Copy link
Contributor

Thanks for the improvements @yoshokatana 👍

@byronhulcher
Copy link
Contributor

WHOAH

@byronhulcher
Copy link
Contributor

WHOAH WHOAH WOOOOOOOAH

@nelsonpecora
Copy link
Contributor Author

@byronhulcher @cruzanmo should we revert this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Limit Component List Include To Specific Sites
4 participants