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

feat(storefront): bctheme-1063 modify stencil to start locally with components ui library #895

Merged
merged 1 commit into from
Jun 20, 2022

Conversation

bc-alexsaiannyi
Copy link
Contributor

What?

With this PR we would like to add opportunity to use templates or styles via external BigCommerce Lib( it will be created later). For this purpose we are going to implement a rule that indicates it's been taken from the external package and not from a directory within a Theme.
For instance:
{{> external/bc-ui-lib/templates/carousel-content }} external/ is a flag that template comes from some external lib.
while
{{> components/common/carousel-content}}. is a default way to use html component within a Theme.

Tickets / Documentation

Screenshots (if appropriate)

TBD

cc @bigcommerce/storefront-team

@bc-alexsaiannyi bc-alexsaiannyi added the draft Ready for initial review label Mar 23, 2022
Copy link
Contributor

@jairo-bc jairo-bc left a comment

Choose a reason for hiding this comment

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

Generally looks good. Left few suggestions. Also would you mind update Draft section in the changelog?

* @param templateName
*/
function getCustomPath(templatesFolder, templateName) {
const packageFlag = 'external/';
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe it's not a flag. A flag is usually considerer a boolean variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's changed to packageMarker since it indicates which source has been used for template.
@jairo-bc how do you find this name?

* Takes a templates folder and template name. It returns simple path for custom template if it's available
* or use default folder instead
*
* @param templatesFolder
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mind adding a type here and below?

@jairo-bc
Copy link
Contributor

@bc-alexsaiannyi @BC-krasnoshapka I believe it's just a stencil cli part. Are you going to update storefront-renderer as well or it will be our work?

@jairo-bc
Copy link
Contributor

is it @bc-alexsaiannyi fully tested and ready to be merged?

@BC-krasnoshapka
Copy link

Let's wait with merging. I believe we should continue with stencil bundle and storefront-renderer parts, and then merge everything when whole idea proves its viability.

@bc-alexsaiannyi bc-alexsaiannyi merged commit e66c57a into bigcommerce:master Jun 20, 2022
@jairo-bc jairo-bc mentioned this pull request Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draft Ready for initial review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants