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

Allow strings to be replaced with an empty string. Changes within strReplace file #256

Closed

Conversation

partner91
Copy link

What? Why?

Added check for an empty string to be eligible to change the old string. This feature would allow you to easily manipulate prices within BC templates because you have to strip currency sign commas etc.

How was it tested?

This feature was tested on the development BigCommerce theme and it works as expected.


cc @bigcommerce/storefront-team

@jairo-bc
Copy link
Contributor

@partner91 do you mind providing unit tests for your improvement that covers before and after cases?

@partner91
Copy link
Author

partner91 commented Mar 20, 2023

In spec/helpers/strReplace.js line:33 there is a test for an empty string, somehow it runs without throwing an error. But without adding check that I've posted you can't replace string with an empty character. You can try running all tests. Below you can find the test I'm referring to.

it('should remove characters from string', function(done) {
runTestCases([
{
input: '{{strReplace "123-45-6789" "-" ""}}',
output: '123456789',
},
], done);
});

@jairo-bc
Copy link
Contributor

Hi @partner91, this feature should be addressed by #259. Could you please verify at your end?

@jairo-bc
Copy link
Contributor

Closing this since it was fixed in #259

@jairo-bc jairo-bc closed this Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants