Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix escaping in group names, no results message. #2879

Merged
merged 2 commits into from
Sep 1, 2017
Merged

Conversation

adunkman
Copy link
Contributor

@harvesthq/chosen-developers

Surprisingly, I don’t think I found any issues in this repo for this one — but we’re incorrectly escaping group names and the no results message in some cases:

  • Group name is not properly escaped when it contains a highlighted search term.
  • The user-entered text is never properly escaped in the no results message.
Before this PR After this PR Status
image image (no issue)
image image Fixed
image image (no issue)
image image Fixed
image image (no issue)

Test cases included.

@tjschuck
Copy link
Member

That before/after table is a thing of beauty, @adunkman 😻

Copy link
Contributor

@satchmorun satchmorun left a comment

Choose a reason for hiding this comment

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

Be still my beating ❤️ !

This kind of PR just makes me happy. Eminently sensible, bugs fixed, things in a better state, and a production code diff full of red lines.

@@ -28,3 +28,72 @@ describe "Searching", ->

results = div.select(".active-result")
expect(results.length).toBe(0)


Copy link
Member

Choose a reason for hiding this comment

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

✂️

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants