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: adding custom blocks test #4

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

subhendukundu
Copy link

Adding this custom block-helpers, as well inbuilt helpers like #with for handlebars.

@ThaNarie
Copy link
Member

Hi @subhendukundu , thanks for the addition, would you be able to add unit test fixtures for this as well? :)

@subhendukundu
Copy link
Author

@ThaNarie , I have added the test cases, let me know what do you think?

@subhendukundu
Copy link
Author

Are we merging this anytime soon?

@@ -0,0 +1 @@
<sly data-sly-test="${ author }"><h2>By ${ firstName } ${ lastName }</h2></sly>
Copy link
Member

Choose a reason for hiding this comment

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

It this correct? In hbs the name fields are nested within the with block, so without changing scope in htl, shouldn't it be author.firstName ?

@@ -0,0 +1 @@
<sly data-sly-test="${ foo == 'false' }">Foo</sly>
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be false as a boolean, currently you're comparing it to a string?

@@ -0,0 +1 @@
{{#gt noListItems 1}}<h2>Greater</h2>{{/gt}}
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a dedicated custom helper for this?
In Muban we have https://github.com/mediamonks/muban/blob/master/build-tools/handlebars-helpers/condition.js, where you can use all condition types. Does > work in htl in a -test check?

@@ -0,0 +1 @@
{{#is noListItems 1}}Foo{{/is}}
Copy link
Member

Choose a reason for hiding this comment

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

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.

2 participants