fix(PM-3398): qa feedbacks and added rich text support for description field #1408
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| }) | ||
| } | ||
|
|
||
| function handleDescriptionChange(value: string): void { |
There was a problem hiding this comment.
[❗❗ security]
The handleDescriptionChange function directly updates the formValues state with the new description value. Ensure that this function is only called with sanitized input to prevent potential XSS vulnerabilities, especially since the input is rich text.
| tabIndex={0} | ||
| onChange={bind(handleFormValueChange, this, 'description')} | ||
| onChange={handleDescriptionChange} | ||
| toolbar='undo redo | formatselect | bold italic underline strikethrough | link | alignleft aligncenter alignright alignjustify | numlist bullist outdent indent | table | removeformat' |
There was a problem hiding this comment.
[💡 design]
The toolbar configuration for FieldHtmlEditor includes several formatting options. Consider limiting the toolbar options to only those necessary for the application's use case to enhance user experience and reduce potential misuse.
...s/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.module.scss
Show resolved
Hide resolved
...s/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.module.scss
Show resolved
Hide resolved
...s/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.module.scss
Show resolved
Hide resolved
src/apps/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.tsx
Show resolved
Hide resolved
| </span> | ||
| ) | ||
| })} | ||
| {props.work.associatedSkills.map((skillId: string) => props.skillNamesMap?.[skillId] || skillId).join(', ')} |
There was a problem hiding this comment.
[💡 maintainability]
The use of Array.prototype.map followed by join(', ') is correct but could be more readable if extracted into a separate function or variable. This would improve maintainability and readability by clearly separating the transformation logic from the rendering logic.
src/libs/shared/lib/components/field-html-editor/BundledEditor/BundledEditor.tsx
Show resolved
Hide resolved
...iles/src/member-profile/work-expirence/ModifyWorkExpirenceModal/ModifyWorkExpirenceModal.tsx
Show resolved
Hide resolved
src/apps/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.tsx
Show resolved
Hide resolved
| dangerouslySetInnerHTML={{ | ||
| __html: DOMPurify.sanitize(props.work.description, { | ||
| ALLOWED_ATTR: ['href', 'target', 'rel'], | ||
| ALLOWED_TAGS: ['p', 'br', 'strong', 'em', 'u', 's', 'strike', 'ul', 'ol', 'li', 'a'], |
There was a problem hiding this comment.
[design]
Adding 's' and 'strike' to the ALLOWED_TAGS list is technically correct, but be cautious as these tags are often used for strikethrough text, which could be misleading or misused in user-generated content. Ensure that this change aligns with the intended use case and consider any potential implications on content clarity or user experience.
...s/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.module.scss
Show resolved
Hide resolved
...s/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.module.scss
Show resolved
Hide resolved
src/apps/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.tsx
Show resolved
Hide resolved
src/apps/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.tsx
Show resolved
Hide resolved
src/apps/profiles/src/member-profile/work-expirence/WorkExpirenceCard/WorkExpirenceCard.tsx
Show resolved
Hide resolved
Related JIRA Ticket:
https://topcoder.atlassian.net/browse/PM-3398
What's in this PR?