-
Notifications
You must be signed in to change notification settings - Fork 4
86b7h9kmt - fix: replace hidden html string for empty string #738
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
86b7h9kmt - fix: replace hidden html string for empty string #738
Conversation
|
@smarcet Don't forget about this PR. |
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.
@niko-exo please review commnets
0237caf to
1b497e8
Compare
📝 WalkthroughWalkthroughThe Formik text editor's onChange now routes the editor HTML through a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Editor as JoditEditor
participant Norm as normalizeJoditEmpty
participant Formik
User->>Editor: type / edit content
Editor->>Editor: produce HTML string (e.g. "<p><br></p>")
Editor->>Norm: onChange -> pass HTML
Norm-->>Editor: return "" or original HTML
Editor->>Formik: setFieldValue(normalizedValue)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
|
Rebase based on master performed on branch and changes force pushed. |
| id={name} | ||
| value={values[name]} | ||
| onChange={(e) => setFieldValue(name, e.target.value)} | ||
| onChange={(e) => { |
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.
@niko-exo seems that comment got truncated
let move this to an util method and lets take care of following corner cases
const normalizeJoditEmpty = (html) => { if (!html) return ""; const v = html.trim(); return (v === "<p><br></p>" || v === "<p><br/></p>" || v === "<p> </p>" || v === "<p></p>") ? "" : html; };
|
@smarcet Checkout the new implementation I've done. Seems to cover all cases you mentioned and more. |
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.
LGTM
ref: https://app.clickup.com/t/86b7h9kmt
86b7h9kmt - fix: replace hidden html string for empty string
Changelog
Links
86b7h9kmt - User is able to save without filling out mandatory field
Evidence
2025-12-24_11-06-02.mp4
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.