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

Fixes an issue where passing in customProperties would render invalidly encoded object #1154

Closed
wants to merge 4 commits into from

Conversation

sdglhm
Copy link

@sdglhm sdglhm commented May 14, 2024

Description

This fixes an issue where when passing in customProperties to a Choice would lead to render [object Object] instead of properly rendering JSON structure.

Types of changes

  • Chore (tooling change or documentation change)
  • Refactor (non-breaking change which maintains existing functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • My code follows the code style of this project.
  • I have added new tests for the bug I fixed/the new feature I added.
  • I have modified existing tests for the bug I fixed/the new feature I added.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@@ -321,7 +321,7 @@ const templates = {
const opt = new Option(label, value, false, active);

if (customProperties) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI, but this isn't a sufficient test for if a javascript object is empty

Copy link
Author

Choose a reason for hiding this comment

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

Thank you for the feedback. I'll fix the tests and update the PR.

Copy link
Author

Choose a reason for hiding this comment

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

@Xon I've updated the tests. Could you please let me know if this is correct. I'm not really great with JS as I majorly work on backends. Thank you.

@sdglhm sdglhm requested a review from Xon July 22, 2024 11:51
@Xon
Copy link
Collaborator

Xon commented Aug 6, 2024

Thanks, I've implemented a more comprehensive fix as part of #1166

@Xon Xon closed this Aug 6, 2024
@sdglhm sdglhm deleted the bugfix/fix-custom-properties branch August 6, 2024 03:23
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