-
Notifications
You must be signed in to change notification settings - Fork 142
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
feat(storefront): bctheme-1063 modify stencil to start locally with components ui library #895
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.
Generally looks good. Left few suggestions. Also would you mind update Draft section in the changelog?
lib/template-assembler.js
Outdated
* @param templateName | ||
*/ | ||
function getCustomPath(templatesFolder, templateName) { | ||
const packageFlag = 'external/'; |
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.
I believe it's not a flag. A flag
is usually considerer a boolean variable.
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.
it's changed to packageMarker
since it indicates which source has been used for template.
@jairo-bc how do you find this name?
lib/template-assembler.js
Outdated
* 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 |
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.
Do you mind adding a type here and below?
@bc-alexsaiannyi @BC-krasnoshapka I believe it's just a |
f4160b0
to
12ce05c
Compare
is it @bc-alexsaiannyi fully tested and ready to be merged? |
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. |
…omponents ui library
12ce05c
to
eccacb9
Compare
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