-
Notifications
You must be signed in to change notification settings - Fork 4
fix: sponsor form description/instructions validation for html #775
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
base: master
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a HTML-normalizing required validator and replaces string-required validations with the new HTML-aware validator for several form fields and the rich-text editor normalization function. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/yup.js`:
- Around line 90-104: The transform in requiredHTMLValidation currently strips
tags but leaves HTML entities like so strings like "<p> </p>" pass
validation; update the normalization to convert non-breaking spaces to regular
spaces before trimming—either modify stripHtmlTags or the transform in
requiredHTMLValidation to replace occurrences of " " and " " (or
convert \u00A0) with a normal space, then strip tags and call .trim(); ensure
you reference stripHtmlTags and requiredHTMLValidation so the transform returns
a truly empty string for content that only contains NBSPs.
src/utils/yup.js
Outdated
| .transform((value, originalValue) => { | ||
| // If the value is a string, strip HTML tags | ||
| if (typeof originalValue === "string") { | ||
| const strippedValue = stripHtmlTags(originalValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque can we use this helper method here https://github.com/fntechgit/summit-admin/blob/cd303a70610bfda34d21ed868870aa3a1edc7e59/src/components/inputs/utils/normalizeJoditEmpty.js ? instead of stripHtmlTags?
smarcet
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@santipalenque please review comments
97b2bdc to
5e468d2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/yup.js`:
- Around line 90-105: The transform in requiredHTMLValidation currently strips
tags but leaves HTML entities like so strings of entities can pass
validation; update either stripHtmlTags or the transform callback in
requiredHTMLValidation to first normalize HTML entities to regular whitespace
(e.g., replace common entities such as and numeric entities like  
or use a generic regex /&[^\s;]+;/g → ' ') before stripping tags and trimming,
ensuring entity-only content becomes empty and fails the required check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/utils/normalize-html-string/index.js`:
- Around line 1-4: normalizeHtmlString currently checks
doc.body.textContent.length === 0 which treats whitespace and non-breaking
spaces as content; update the check to normalize whitespace by trimming and
replacing NBSPs (e.g., replace \u00A0 with regular space) and collapsing
whitespace before measuring length. Specifically, in normalizeHtmlString use new
DOMParser().parseFromString(textInput, "text/html") to get doc, compute a
normalizedText from doc.body.textContent by replacing \u00A0 with ' ',
collapsing consecutive whitespace to a single space and trimming, then return ""
if normalizedText.length === 0 otherwise return textInput.
| // removes html tags if contained text is empty | ||
| const normalizeHtmlString = (textInput) => { | ||
| const doc = new DOMParser().parseFromString(textInput, "text/html"); | ||
| return doc.body.textContent.length === 0 ? "" : textInput; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Treat whitespace-only HTML as empty.
textContent.length === 0 will treat whitespace or as content, so required HTML validation can pass with visually empty input. Normalize whitespace before checking length.
🛠️ Proposed fix
- return doc.body.textContent.length === 0 ? "" : textInput;
+ const text = doc.body.textContent.replace(/\u00a0/g, " ").trim();
+ return text.length === 0 ? "" : textInput;🤖 Prompt for AI Agents
In `@src/utils/normalize-html-string/index.js` around lines 1 - 4,
normalizeHtmlString currently checks doc.body.textContent.length === 0 which
treats whitespace and non-breaking spaces as content; update the check to
normalize whitespace by trimming and replacing NBSPs (e.g., replace \u00A0 with
regular space) and collapsing whitespace before measuring length. Specifically,
in normalizeHtmlString use new DOMParser().parseFromString(textInput,
"text/html") to get doc, compute a normalizedText from doc.body.textContent by
replacing \u00A0 with ' ', collapsing consecutive whitespace to a single space
and trimming, then return "" if normalizedText.length === 0 otherwise return
textInput.
https://app.clickup.com/t/86b8b1nup
Summary by CodeRabbit