Skip to content

Conversation

@shreya-kamble
Copy link
Contributor

No description provided.

@shreya-kamble shreya-kamble requested a review from a team as a code owner March 19, 2025 06:47
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for retaining empty attribute values during HTML conversion by introducing the allowedEmptyAttributes option. Key changes include:

  • Enhancing toRedactor conversion logic to merge allowedEmptyAttributes from options.
  • Extending type definitions and updating documentation to support the new option.
  • Adding tests and expected output updates for the new RT-268 behavior.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
test/expectedJson.ts Added expected JSON and HTML for RT-268 scenarios.
src/toRedactor.tsx Introduced ALLOWED_EMPTY_ATTRIBUTES constant and updated attribute merging logic.
README.md Updated documentation to include allowedEmptyAttributes details.
src/types.ts Extended type definitions to include IJsonToHtmlAllowedEmptyAttributes.
test/toRedactor.test.ts Added new tests to verify the allowed empty attributes functionality.
Comments suppressed due to low confidence (2)

src/toRedactor.tsx:228

  • [nitpick] Merging allowedEmptyAttributes arrays without filtering duplicates might lead to redundant attribute values. Consider using a Set or filtering duplicates when combining the arrays.
ALLOWED_EMPTY_ATTRIBUTES[key] = [ ...ALLOWED_EMPTY_ATTRIBUTES[key], ...(options.allowedEmptyAttributes?.[key] || []) ];

src/toRedactor.tsx:527

  • [nitpick] Using 'return' within a forEach callback to control loop continuation may reduce clarity. Consider refactoring the loop with a traditional for-loop or explicit if/else constructs for improved readability.
Object.entries(attrsJson).forEach((item) => {

Copy link
Contributor

@Jayesh2812 Jayesh2812 left a comment

Choose a reason for hiding this comment

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

Looks Good To Me.

@shreya-kamble shreya-kamble merged commit f46a874 into dev Mar 19, 2025
9 checks passed
shreya-kamble added a commit that referenced this pull request Mar 24, 2025
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.

3 participants