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

fix: improve HTML content handling in algoliaSplitter.js #216

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

Conversation

htuzel
Copy link
Collaborator

@htuzel htuzel commented Dec 19, 2024

What is Changed

  • Refactored splitHtmlContent to enhance section trimming and ensure empty sections are properly handled.
  • Updated removeIgnoredContent to correctly remove multiline content within ignored tags and self-closing tags.
  • Added a new test case to verify removal of content inside multiline tags.

- Refactored `splitHtmlContent` to enhance section trimming and ensure empty sections are properly handled.
- Updated `removeIgnoredContent` to correctly remove multiline content within ignored tags and self-closing tags.
- Added a new test case to verify removal of content inside multiline `<area>` tags.
@htuzel htuzel requested a review from sbellone December 19, 2024 16:38
@htuzel htuzel self-assigned this Dec 19, 2024
Comment on lines 34 to 36
section = StringUtils.trim(splitterBegin + section) || '';

if (!section.trim()) {
if (!section) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

section can't be undefined: splitterBegin is initialized manually at the beginning of the function.

new RegExp('<' + tag + '[^>]*>[\\s\\S]*?<\\/' + tag + '>', 'gi'),
''
);
// Remove self-closing tags <tag ... />
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good, I think this addresses this comment thanks: https://github.com/algolia/algoliasearch-sfcc-b2c/pull/164/files#r1565893249

Please add unit tests on the removeIgnoredContent function directly to confirm, including tests with self-closing tags.

- Expanded unit tests in `algoliaSplitter.test.js` to cover various scenarios for the new function, ensuring robust handling of ignored tags and preserving relevant content.
@htuzel htuzel requested a review from sbellone January 16, 2025 14:39
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