Story: 2437 User Profile Edit UI#2478
Merged
Merged
Conversation
julioest
approved these changes
Jun 5, 2026
herzog0
requested changes
Jun 9, 2026
herzog0
left a comment
herzog0
left a comment
Collaborator
There was a problem hiding this comment.
Heya! Thanks for this, the UI is looking great :D
Just a few things I think should be adjusted before shipping
Collaborator
There was a problem hiding this comment.
Heya! Is there any specific reason we need to duplicate the wysiwyg component here?
I also noticed there are a lot of imports not being used and a few function calls/refs that don't seem to exist in the scope of the script (e.g. isSafeUrl, parseMarkdownSafe and others..).
Collaborator
Author
There was a problem hiding this comment.
So this was the best solution I found for the requirements of limiting the features on the wysiwig. I'm not super happy with this solution, but it is the best one I discovered.
Collaborator
There was a problem hiding this comment.
Sure thing, no problem with that specifically. My comment is more related to the unused imports and the missing function references. Did you have the chance to take a look at those?
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a complete v3 user profile edit interface featuring a TipTap-based WYSIWYG editor with Markdown support, comprehensive form field templates, backend form definitions for profile links and commit emails, view routing logic, a two-column layout template, supporting CSS/design system enhancements for forms and avatars, and automatic editor initialization in news templates. ChangesUser Profile Edit Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/wysiwyg-editor.js (1)
35-36:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winExport additional functions required by user-profile-wysiwyg-editor.js.
The user-profile editor (imported at line 19 of
user-profile-wysiwyg-editor.js) calls several functions from this module that are not exported:
parseMarkdownSafe(line 35) - called at lines 157, 199, 239, 267, 287turndown(line 53) - used at lines 253, 308isSafeUrl(line 87) - called at line 79openModal(line 96) - called at line 75highlightPreviewCodeBlocks(line 655) - called at lines 240, 288renderMermaidPreview(line 669) - called at lines 241, 289Without exporting these, the user-profile editor will crash with
ReferenceErrorat runtime.📤 Proposed fix
-function parseMarkdownSafe(md) { +export function parseMarkdownSafe(md) { return DOMPurify.sanitize(marked.parse(md)); } -function sanitizeSvg(svgString) { +export function sanitizeSvg(svgString) { return DOMPurify.sanitize(svgString, { USE_PROFILES: { svg: true, svgFilters: true }, ADD_TAGS: ["use"] }); } // ... later in file ... -const turndown = new TurndownService({ +export const turndown = new TurndownService({ headingStyle: "atx", // ... later in file ... -const isSafeUrl = (url) => { +export const isSafeUrl = (url) => { try { // ... later in file ... -const openModal = (title, fields) => +export const openModal = (title, fields) => new Promise((resolve) => { // ... later in file ... -const highlightPreviewCodeBlocks = (container) => { +export const highlightPreviewCodeBlocks = (container) => { container.querySelectorAll("pre code[class*='language-']").forEach((codeEl) => { // ... later in file ... -const renderMermaidPreview = async (container) => { +export const renderMermaidPreview = async (container) => { const mermaidCodes = container.querySelectorAll("code.language-mermaid");Also applies to: 53-62, 87-94, 96-164, 655-667, 669-695
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/wysiwyg-editor.js` around lines 35 - 36, The module currently defines utility functions used by user-profile-wysiwyg-editor.js but doesn't export them, causing runtime ReferenceErrors; update the file to export the required functions—add named exports for parseMarkdownSafe, turndown, isSafeUrl, openModal, highlightPreviewCodeBlocks, and renderMermaidPreview (or export them together at the bottom with export { parseMarkdownSafe, turndown, isSafeUrl, openModal, highlightPreviewCodeBlocks, renderMermaidPreview };), ensuring the functions referenced (parseMarkdownSafe, turndown, isSafeUrl, openModal, highlightPreviewCodeBlocks, renderMermaidPreview) remain defined and available to imports.templates/v3/includes/_avatar_v3.html (1)
31-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd explicit intrinsic dimensions for
size="xxxl"images.Line 31 still falls back to
40x40forxxxl, while CSS renders112x112. Add anxxxlbranch so intrinsic size matches rendered size.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@templates/v3/includes/_avatar_v3.html` at line 31, The img tag rendering avatar intrinsic dimensions falls back to 40x40 for size=="xxxl"; update the width and height template expressions in the <img> that produces class "avatar__img" to add an elif branch for size == 'xxxl' and return 112 for both width and height so the intrinsic dimensions match the CSS-rendered 112x112.
♻️ Duplicate comments (2)
frontend/user-profile-wysiwyg-editor.js (1)
1-19:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove unused imports.
Several imports are not used in this file:
- Line 19:
handleKeyDownandhandlePasteare imported but never called (reimplemented inline at lines 177-187 and 188-207)- Lines 3, 6-10: TipTap extensions
CodeBlockLowlight,Table,TableRow,TableCell,TableHeader, andImageare imported but never configured in the editor (lines 166-171 only use StarterKit, Underline, TaskList, TaskItem)🧹 Proposed fix
import { Editor } from "`@tiptap/core`"; import StarterKit from "`@tiptap/starter-kit`"; -import CodeBlockLowlight from "`@tiptap/extension-code-block-lowlight`"; import Underline from "`@tiptap/extension-underline`"; -import Link from "`@tiptap/extension-link`"; -import Table from "`@tiptap/extension-table`"; -import TableRow from "`@tiptap/extension-table-row`"; -import TableCell from "`@tiptap/extension-table-cell`"; -import TableHeader from "`@tiptap/extension-table-header`"; -import Image from "`@tiptap/extension-image`"; import TaskList from "`@tiptap/extension-task-list`"; import TaskItem from "`@tiptap/extension-task-item`"; -import { common, createLowlight } from "lowlight"; -import { toHtml } from "hast-util-to-html"; -import { marked } from "marked"; -import DOMPurify from "dompurify"; -import TurndownService from "turndown"; -import { gfm } from "turndown-plugin-gfm"; -import {handleKeyDown, handlePaste, createToolbarButton, ICONS, setupMermaidEditMode, debounce} from './wysiwyg-editor'; +import { createToolbarButton, ICONS, setupMermaidEditMode, debounce } from './wysiwyg-editor';🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/user-profile-wysiwyg-editor.js` around lines 1 - 19, Remove the unused imports or wire them up: either delete the unused symbols (CodeBlockLowlight, Table, TableRow, TableCell, TableHeader, Image, handleKeyDown, handlePaste) from the top of frontend/user-profile-wysiwyg-editor.js, or instead reuse the shared implementations by calling the imported handleKeyDown and handlePaste functions where inline handlers are currently implemented (lines ~177-207) and add the missing TipTap extensions (CodeBlockLowlight, Table, TableRow, TableCell, TableHeader, Image) into the Editor configuration (the array passed to new Editor / extensions at ~166-171) so they are actually used; update imports accordingly to keep only what the file uses.users/forms.py (1)
207-227:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInstantiate nested formsets per
V3UserProfileForminstance, not as class attributes.
link_formsetandcommit_email_formsetare currently created at class scope (Line 239 and Line 282), andif commit_emails:(Line 218) skips rebuilding when an empty list is passed. That leaves shared/default state in place and will produce incorrect per-user formset state.Suggested fix
class V3UserProfileForm(forms.Form): def __init__(self, *args, **kwargs): - links = kwargs.pop("user_links", None) - commit_emails = kwargs.pop("commit_emails", None) + links = kwargs.pop("user_links", None) or {} + commit_emails = kwargs.pop("commit_emails", None) or [] super().__init__(*args, **kwargs) - if links: - self.link_formset = V3ProfileLinkFormset( - initial=[ - {"type": x, "value": links.get(x, "")} - for x in V3ProfileLinkChoices.values - ], - ) - if commit_emails: - self.commit_email_formset = V3CommitEmailFormSet( - initial=[ - { - "email": x, - } - for x in commit_emails - ], - ) + self.link_formset = V3ProfileLinkFormset( + initial=[ + {"type": x, "value": links.get(x, "")} + for x in V3ProfileLinkChoices.values + ] + ) + self.commit_email_formset = V3CommitEmailFormSet( + initial=[{"email": x} for x in commit_emails] + ) @@ - link_formset = V3ProfileLinkFormset( - initial=[{"type": x, "value": ""} for x in V3ProfileLinkChoices.values], - ) @@ - commit_email_formset = V3CommitEmailFormSet( - initial=[ - { - "email": "abc@example.com", - }, - ], - )Also applies to: 239-241, 282-288
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@users/forms.py` around lines 207 - 227, The V3UserProfileForm currently leaves link_formset and commit_email_formset as class-level/shared defaults and skips rebuilding when an empty list is passed; in V3UserProfileForm.__init__ always assign instance attributes self.link_formset and self.commit_email_formset (not rely on class attributes) and construct them unconditionally or when the corresponding kwargs is not None (use "if commit_emails is not None" to allow empty lists), creating V3ProfileLinkFormset (using V3ProfileLinkChoices.values for initial) and V3CommitEmailFormSet (using commit_emails for initial) so each form instance has its own fresh formsets instead of shared state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/user-profile-wysiwyg-editor.js`:
- Around line 89-91: The function handleDocClick in buildToolbar references
undefined variables tableWrapper and gridPopup; remove this dead code (delete
the handleDocClick definition and any registrations like
document.addEventListener('click', handleDocClick)) or, if behavior is required,
replace it with a safe implementation that uses existing toolbar elements
instead; ensure no leftover event listeners or references to
tableWrapper/gridPopup remain in buildToolbar.
- Line 21: Remove the unused lowlight initialization: delete the const lowlight
= createLowlight(common) line (and remove the createLowlight and common imports
if they are no longer used) because CodeBlockLowlight is not included in this
editor configuration; ensure no other references to lowlight or createLowlight
remain in the file (search for lowlight, createLowlight, and common) and run the
linter/TypeScript checks to confirm no unused-import errors.
- Around line 75-87: The Link action uses openModal and isSafeUrl but those
helpers are only defined (const isSafeUrl, const openModal) in another module
without exports, causing runtime failures; either export them from
frontend/wysiwyg-editor.js and import them into this file, or implement
equivalents locally in frontend/user-profile-wysiwyg-editor.js, then update the
Link command to call the imported/local openModal and isSafeUrl before calling
editor.chain().focus().setLink(...). Ensure the exported names match exactly:
openModal and isSafeUrl.
- Around line 157-160: The file references undefined symbols parseMarkdownSafe,
highlightPreviewCodeBlocks, renderMermaidPreview, and turndown causing
ReferenceErrors; fix by exporting those helpers (or a single helper that returns
turndown) from frontend/wysiwyg-editor.js and importing them into
frontend/user-profile-wysiwyg-editor.js, or alternatively implement equivalent
local versions in user-profile-wysiwyg-editor.js that match the behavior used
around initialContent parsing and preview rendering (update the catch block
usage of parseMarkdownSafe, the preview calls to
highlightPreviewCodeBlocks/renderMermaidPreview, and any turndown
instantiation/usage to use the imported or locally defined identifiers).
In `@templates/v3/includes/_field_checkbox.html`:
- Around line 16-21: Change the hard-coded id/for pair to use an optional
override variable so IDs can be unique when the partial is reused: update the
label's for and the input's id from "checkbox-{{ name }}" to use something like
"{{ input_id or ('checkbox-' ~ name) }}", so the template will accept an
explicit input_id (unique per option) but fall back to the existing name-based
id when not provided; then in multi-checkbox callers (e.g.,
checkboxselectmultiple loops) pass a unique input_id (forloop.counter0 or
similar) for each option.
In `@templates/v3/includes/_field_dropdown.html`:
- Around line 103-106: The dropdown trigger element with class
"dropdown__trigger" and role="button" currently keeps tabindex="0" even when the
template variable disabled is true; update the template so that when disabled is
true the element is not tabbable (remove tabindex or set tabindex="-1") and add
aria-disabled="true" to indicate non-interactive state for assistive tech; apply
the same change to the other occurrence around the second trigger (the block
referenced at line ~115) so both disabled triggers are non-focusable and
properly exposed as disabled.
- Line 95: The Alpine :disabled expression currently renders Python True/False
and breaks JS logic; change the serialization of the Django `disabled` context
used in the `:disabled` attribute to emit lowercase `true`/`false` (e.g. use the
template filter like `{{ disabled|default:"false"|lower }}` or `{{
disabled|yesno:"true,false" }}`) so the expression becomes `jsReady ||
true/false`; additionally, when the trigger is disabled ensure it is not
focusable and exposes state: add aria-disabled="true" and set tabindex="-1" when
`disabled` is true, and remove/restore tabindex/aria-disabled when false (update
the same template element that contains the `:disabled` attribute).
In `@templates/v3/includes/_field_include.html`:
- Line 15: The checkbox include is using bound_field.initial as the value which
doesn't set the checked state; update the include invocation in
_field_include.html to pass the current boolean state (bound_field.value) as the
checked argument (e.g., include 'v3/includes/_field_checkbox.html' with ...
checked=bound_field.value ...) so the checkbox reflects the bound field's actual
value; keep other parameters (name, label, required, disabled, help_text)
unchanged.
In `@templates/v3/user_profile_edit.html`:
- Line 28: The WYSIWYG include is wired to textarea_name='content' and
textarea_id='id_content' but the form field is user_profile_form.bio, so update
the include invocation (the call to v3/includes/_wysiwyg_editor.html) to use
textarea_name='bio' and textarea_id='id_bio' so the editor binds to
user_profile_form.bio; also search for any client-side code that references
id_content and update it to id_bio to keep JS submit/wiring working.
- Line 121: The included bound field name is misspelled: change the include that
references user_profile_form.ovverride_commit_author_email to use the correct
field name user_profile_form.override_commit_author_email so the template
renders and wires the override_commit_author_email field properly (update the
bound_field in the include call).
In `@users/forms.py`:
- Around line 250-261: These BooleanField profile checkboxes currently validate
as required and cause errors when left unchecked; update the field definitions
for hide_github, hide_ml, hide_ach, indicate_last_login_method,
override_commit_author_name, override_commit_author_email, and
allow_notification_terms_updated in users/forms.py to include required=False so
unchecked boxes are accepted as False, and keep existing labels/help_text
unchanged to preserve behavior.
In `@users/views.py`:
- Around line 108-109: The current check self.request.GET.get("edit", False)
treats any non-empty value (e.g. ?edit=false) as true; change to an explicit
parse such as reading val = self.request.GET.get("edit") and enabling edit mode
only when val is exactly "true" (case-insensitive) or when you intentionally
want presence semantics use "if 'edit' in self.request.GET" — update the
V3UserProfileForm branch and duplicate the same fix at the other occurrence (the
check around lines 419-420) so both locations use the same explicit comparison
(e.g., val and val.lower() == "true" or presence check).
---
Outside diff comments:
In `@frontend/wysiwyg-editor.js`:
- Around line 35-36: The module currently defines utility functions used by
user-profile-wysiwyg-editor.js but doesn't export them, causing runtime
ReferenceErrors; update the file to export the required functions—add named
exports for parseMarkdownSafe, turndown, isSafeUrl, openModal,
highlightPreviewCodeBlocks, and renderMermaidPreview (or export them together at
the bottom with export { parseMarkdownSafe, turndown, isSafeUrl, openModal,
highlightPreviewCodeBlocks, renderMermaidPreview };), ensuring the functions
referenced (parseMarkdownSafe, turndown, isSafeUrl, openModal,
highlightPreviewCodeBlocks, renderMermaidPreview) remain defined and available
to imports.
In `@templates/v3/includes/_avatar_v3.html`:
- Line 31: The img tag rendering avatar intrinsic dimensions falls back to 40x40
for size=="xxxl"; update the width and height template expressions in the <img>
that produces class "avatar__img" to add an elif branch for size == 'xxxl' and
return 112 for both width and height so the intrinsic dimensions match the
CSS-rendered 112x112.
---
Duplicate comments:
In `@frontend/user-profile-wysiwyg-editor.js`:
- Around line 1-19: Remove the unused imports or wire them up: either delete the
unused symbols (CodeBlockLowlight, Table, TableRow, TableCell, TableHeader,
Image, handleKeyDown, handlePaste) from the top of
frontend/user-profile-wysiwyg-editor.js, or instead reuse the shared
implementations by calling the imported handleKeyDown and handlePaste functions
where inline handlers are currently implemented (lines ~177-207) and add the
missing TipTap extensions (CodeBlockLowlight, Table, TableRow, TableCell,
TableHeader, Image) into the Editor configuration (the array passed to new
Editor / extensions at ~166-171) so they are actually used; update imports
accordingly to keep only what the file uses.
In `@users/forms.py`:
- Around line 207-227: The V3UserProfileForm currently leaves link_formset and
commit_email_formset as class-level/shared defaults and skips rebuilding when an
empty list is passed; in V3UserProfileForm.__init__ always assign instance
attributes self.link_formset and self.commit_email_formset (not rely on class
attributes) and construct them unconditionally or when the corresponding kwargs
is not None (use "if commit_emails is not None" to allow empty lists), creating
V3ProfileLinkFormset (using V3ProfileLinkChoices.values for initial) and
V3CommitEmailFormSet (using commit_emails for initial) so each form instance has
its own fresh formsets instead of shared state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4f3a887f-0e3b-46eb-816b-1eba3b5f28d3
📒 Files selected for processing (19)
frontend/user-profile-wysiwyg-editor.jsfrontend/wysiwyg-editor.jspackage.jsonstatic/css/v3/avatar.cssstatic/css/v3/buttons.cssstatic/css/v3/forms.cssstatic/css/v3/user-profile-page.cssstatic/css/v3/wysiwyg-editor.cssstatic/js/v3/user-profile-wysiwyg-editor.jstemplates/includes/icon.htmltemplates/v3/includes/_avatar_v3.htmltemplates/v3/includes/_field_checkbox.htmltemplates/v3/includes/_field_dropdown.htmltemplates/v3/includes/_field_include.htmltemplates/v3/includes/_form_include.htmltemplates/v3/includes/_wysiwyg_editor.htmltemplates/v3/user_profile_edit.htmlusers/forms.pyusers/views.py
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/user-profile-wysiwyg-editor.js`:
- Around line 15-61: The setupMermaidEditMode function references three
undefined identifiers that will cause runtime crashes: getMermaid (called within
the renderMermaid handler), mermaidIdCounter (incremented when rendering), and
sanitizeSvg (called to process SVG output). These are defined in
frontend/wysiwyg-editor.js but not exported. Either export all three from
frontend/wysiwyg-editor.js and add import statements at the top of
frontend/user-profile-wysiwyg-editor.js, or implement local equivalents of these
in the current file. Ensure that whichever approach is chosen, all three
identifiers are available in scope before setupMermaidEditMode is called.
In `@static/js/v3/wysiwyg-editor.js`:
- Line 247: The link/image modal created by the iE function lacks proper
semantic attributes and keyboard support. Add role="dialog" and
aria-modal="true" attributes to the modal overlay and modal container elements
respectively. Associate each label with its corresponding input by setting the
label's htmlFor attribute to match the input's id, and ensure each input has a
unique id. Track the previously focused element before the modal opens (store it
before appending to document.body) and restore focus to that element when the
modal closes by calling previousElement.focus() in the cleanup function d.
Implement Tab key trapping within the dialog by capturing Tab keydowns on the
modal, and preventing default behavior while cycling focus between the first and
last focusable elements (the inputs and buttons) using keyboard event handlers.
Additionally, make the table picker cells in the Pv function keyboard accessible
by converting the span elements to buttons with role="button" and implementing
keyboard event listeners to handle Enter and arrow keys for navigation and
selection instead of relying solely on click handlers.
- Line 247: The initialization code in the Uv function uses `n.value.trim()` and
a simplistic HTML detection check (`s.startsWith("<") && s.includes(">")`) that
incorrectly identifies Markdown constructs like `<https://example.com>` as HTML,
and loses meaningful whitespace from the original textarea value. To fix this,
remove the trim() call to preserve the original value's whitespace intact, and
replace the broad HTML detection logic with a more precise check that
distinguishes actual HTML (like `<!DOCTYPE`, `<html>`, `<div>`, etc.) from
Markdown patterns. This ensures content isn't corrupted during initial load and
that meaningful whitespace in code blocks or indentation is preserved through
the edit cycle.
- Line 247: The `dE` function (Mermaid loader) can initiate multiple CDN loads
concurrently if called while a load is already in flight, since it doesn't cache
the in-flight promise, and the debounced render function `n` in `Fv`
(setupMermaidEditMode) can have renders complete out of order, allowing older
diagram previews to overwrite newer ones. Fix this by caching the loader promise
in `dE` so that concurrent calls wait for the same in-flight load to complete,
and add a generation/version tracking mechanism in the debounced render function
in `Fv` so that stale render results are ignored if a newer render has been
initiated since they were queued.
- Line 247: The URL validation function `rE` currently accepts http, https, and
mailto protocols for both links and images, but images should only allow http
and https since mailto URLs create broken img elements. Create two separate
validator functions - one that accepts all three protocols for links and one
that only accepts http and https for images. Update the image insertion handler
(the onClick callback in the image button creation) to use the stricter
validator that rejects mailto, and update the Link extension configuration
(`xm.configure`) to include an `isAllowedUri` validator function that enforces
the same three-protocol policy to prevent bypassing the URL policy through
pasted or imported HTML content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f6e1fcb8-7075-4dda-8628-c801f4a4fcc2
📒 Files selected for processing (4)
frontend/user-profile-wysiwyg-editor.jsfrontend/wysiwyg-editor.jsstatic/js/v3/user-profile-wysiwyg-editor.jsstatic/js/v3/wysiwyg-editor.js
julhoang
requested changes
Jun 15, 2026
julhoang
left a comment
julhoang
left a comment
Collaborator
There was a problem hiding this comment.
Hi @jlchilders11 , thanks for tackling this huge UI task! This is awesome work so far. I've left some styling discrepancies that I noticed here and there and also +1 to all of Teo's comments above 🙏
Comment on lines
+58
to
+60
| {% else %} | ||
| {% include 'v3/includes/_button.html' with style='secondary' type='button' label='Choose File' only %} | ||
| {% endif %} |
Collaborator
There was a problem hiding this comment.
Might we be able to add a form action to upload profile image in this PR, and the floaty "Replace Image" UI section too?
Comment on lines
+69
to
+70
| {% include 'v3/includes/_badge_v3.html' with token='badge-tier-'|add:tier.tier label=tier.name|add:' Badge' size='large' %} | ||
| <span>{{ tier.name }} Badge</span> |
Collaborator
There was a problem hiding this comment.
This badge should have an opacity of 0.45 when it is disabled.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/wysiwyg-editor.js (2)
87-94:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing
isSafeUrlexport causes runtime crash in the Link insertion flow.
isSafeUrlis defined infrontend/wysiwyg-editor.jsbut not exported, whilefrontend/user-profile-wysiwyg-editor.jsreferences it without importing. When a user clicks the Link button on the profile edit page and enters a URL, aReferenceErrorwill be thrown.
frontend/wysiwyg-editor.js#L87-L94: ExportisSafeUrlby changingconst isSafeUrltoexport const isSafeUrl.frontend/user-profile-wysiwyg-editor.js#L100-L113: AddisSafeUrlto the import statement on line 14.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/wysiwyg-editor.js` around lines 87 - 94, The `isSafeUrl` function is defined but not exported in `frontend/wysiwyg-editor.js` (lines 87-94), causing a runtime ReferenceError when referenced in `frontend/user-profile-wysiwyg-editor.js` (lines 100-113). In `frontend/wysiwyg-editor.js`, change `const isSafeUrl` to `export const isSafeUrl` to make the function exportable. In `frontend/user-profile-wysiwyg-editor.js`, add `isSafeUrl` to the import statement on line 14 to properly import the exported function from wysiwyg-editor.js before it is used in the Link insertion flow.
697-702:⚠️ Potential issue | 🔴 CriticalThe removal of the
initWysiwygexport breaks existing consumers that are already in the codebase.The
initWysiwygfunction was exported fromfrontend/wysiwyg-editor.jsbut has been changed to a localconst. However,templates/v3/examples/_v3_example_section_wysiwyg.htmlstill dynamically imports and calls this function at lines 168 and 186:import('/static/js/v3/wysiwyg-editor.js').then(m => m.initWysiwyg('id_demo_content'))This will now fail with
m.initWysiwyg is not a function. Either restore the export or update the HTML template to use the alternativeautoInitpattern instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/wysiwyg-editor.js` around lines 697 - 702, The initWysiwyg function in frontend/wysiwyg-editor.js is declared as a local const but is still being dynamically imported and called by templates/v3/examples/_v3_example_section_wysiwyg.html at lines 168 and 186, which will fail since the function is no longer exported. Either export the initWysiwyg function from frontend/wysiwyg-editor.js to restore the export so it can be imported by the HTML template, or update the HTML template's dynamic import calls at those two locations to use the autoInit pattern instead, whichever approach is preferred for the codebase.
🧹 Nitpick comments (2)
frontend/user-profile-wysiwyg-editor.js (1)
346-358: 💤 Low valueUnescaped
elIdin CSS selector may fail for IDs with special characters.
document.querySelector(\textarea[id=${elId}]`)will break ifelIdcontains CSS special characters like],[, or spaces. UseCSS.escape()orgetElementById()` instead.♻️ Suggested fix using CSS.escape
const autoInit = (elId) => { if (typeof document === "undefined" || !document.querySelector) return; if (elId && elId !== null) { - const ta = document.querySelector(`textarea[id=${elId}]`); + const ta = document.getElementById(elId); if (ta && ta.id) initWysiwyg(ta.id); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/user-profile-wysiwyg-editor.js` around lines 346 - 358, In the autoInit function, the elId parameter is directly interpolated into a CSS selector without escaping, which will cause the querySelector to fail when elId contains special CSS characters like spaces, brackets, or other special characters. Fix this by wrapping the elId with CSS.escape() when constructing the selector string in the querySelector call, so that any special characters in the elId are properly escaped for use in CSS selectors.frontend/wysiwyg-editor.js (1)
904-916: 💤 Low valueSame unescaped selector issue as in
user-profile-wysiwyg-editor.js.
document.querySelector(\textarea[id=${elId}]`)will fail for IDs with special CSS characters. Usedocument.getElementById(elId)` for robustness.♻️ Suggested fix
const autoInit = (elId) => { if (typeof document === "undefined" || !document.querySelector) return; if (elId && elId !== null) { - const ta = document.querySelector(`textarea[id=${elId}]`); + const ta = document.getElementById(elId); if (ta && ta.id) initWysiwyg(ta.id); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/wysiwyg-editor.js` around lines 904 - 916, The querySelector method in the autoInit function is using an unescaped template literal to construct a CSS selector with the elId parameter, which will fail if the ID contains special CSS characters. Replace the document.querySelector call that uses the template literal `textarea[id=${elId}]` with document.getElementById(elId) for a more robust and direct approach to finding the textarea element by its ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/news/form.html`:
- Around line 10-18: The direct calls to `autoInit` will fail with a
ReferenceError because ES6 module exports do not automatically create global
references. In templates/news/form.html lines 10-18 (anchor), replace the direct
`autoInit()` calls in the DOMContentLoaded event listener and the else branch
with a guarded callback that checks `typeof window.autoInit === 'function'`
before invoking it. In templates/news/v3/create.html lines 33-41 (sibling),
apply the same guarded callback pattern for the `autoInit` function calls, and
additionally add `{ once: true }` as the third parameter to the
`addEventListener` call to ensure the listener fires only once.
---
Outside diff comments:
In `@frontend/wysiwyg-editor.js`:
- Around line 87-94: The `isSafeUrl` function is defined but not exported in
`frontend/wysiwyg-editor.js` (lines 87-94), causing a runtime ReferenceError
when referenced in `frontend/user-profile-wysiwyg-editor.js` (lines 100-113). In
`frontend/wysiwyg-editor.js`, change `const isSafeUrl` to `export const
isSafeUrl` to make the function exportable. In
`frontend/user-profile-wysiwyg-editor.js`, add `isSafeUrl` to the import
statement on line 14 to properly import the exported function from
wysiwyg-editor.js before it is used in the Link insertion flow.
- Around line 697-702: The initWysiwyg function in frontend/wysiwyg-editor.js is
declared as a local const but is still being dynamically imported and called by
templates/v3/examples/_v3_example_section_wysiwyg.html at lines 168 and 186,
which will fail since the function is no longer exported. Either export the
initWysiwyg function from frontend/wysiwyg-editor.js to restore the export so it
can be imported by the HTML template, or update the HTML template's dynamic
import calls at those two locations to use the autoInit pattern instead,
whichever approach is preferred for the codebase.
---
Nitpick comments:
In `@frontend/user-profile-wysiwyg-editor.js`:
- Around line 346-358: In the autoInit function, the elId parameter is directly
interpolated into a CSS selector without escaping, which will cause the
querySelector to fail when elId contains special CSS characters like spaces,
brackets, or other special characters. Fix this by wrapping the elId with
CSS.escape() when constructing the selector string in the querySelector call, so
that any special characters in the elId are properly escaped for use in CSS
selectors.
In `@frontend/wysiwyg-editor.js`:
- Around line 904-916: The querySelector method in the autoInit function is
using an unescaped template literal to construct a CSS selector with the elId
parameter, which will fail if the ID contains special CSS characters. Replace
the document.querySelector call that uses the template literal
`textarea[id=${elId}]` with document.getElementById(elId) for a more robust and
direct approach to finding the textarea element by its ID.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2be8655a-ab60-4885-8cc8-e6513d26a71e
📒 Files selected for processing (11)
frontend/user-profile-wysiwyg-editor.jsfrontend/wysiwyg-editor.jsstatic/js/v3/user-profile-wysiwyg-editor.jsstatic/js/v3/wysiwyg-editor.jstemplates/news/form.htmltemplates/news/v3/create.htmltemplates/v3/includes/_field_dropdown.htmltemplates/v3/includes/_field_include.htmltemplates/v3/includes/_wysiwyg_editor.htmltemplates/v3/user_profile_edit.htmlusers/views.py
✅ Files skipped from review due to trivial changes (1)
- templates/v3/includes/_wysiwyg_editor.html
🚧 Files skipped from review as they are similar to previous changes (4)
- templates/v3/includes/_field_include.html
- templates/v3/includes/_field_dropdown.html
- templates/v3/user_profile_edit.html
- users/views.py
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@templates/v3/includes/_avatar_v3.html`:
- Line 31: The img tag in the avatar template has a duplicate class attribute
declaration. The second class attribute overrides the first, causing the size
modifier avatar--{{ size }} to be lost and breaking the avatar sizing CSS. Merge
both class declarations into a single class attribute that includes all three
class names: avatar, avatar--{{ size }}, and avatar__img, separated by spaces.
In `@templates/v3/includes/_field_include.html`:
- Line 17: Remove the `value=bound_field.value` parameter from the include
statement for _field_checkbox.html on line 17. Checkbox inputs use the `checked`
attribute (already present as `checked=bound_field.value`) to control their
boolean state, not the `value` attribute. Passing the Python boolean value
through the `value` parameter renders semantically incorrect HTML like
`value="True"` or `value="False"`. Keep all other parameters (name, label,
required, disabled, help_text, checked) intact and only delete the `value`
parameter and its value.
In `@templates/v3/includes/_wysiwyg_editor.html`:
- Around line 20-33: The wysiwyg-v3 editor template sets jsReady to true
immediately before autoInit() completes asynchronously, causing the fallback
textarea to be hidden via x-show="!jsReady" even if initialization fails. Move
the jsReady = true assignment to execute after autoInit() completes successfully
(likely within a callback or promise chain inside the autoInit() function itself
or in the x-init block after awaiting it), and change the hardcoded
:aria-hidden="true" attribute on the textarea to :aria-hidden="jsReady" so the
fallback textarea only becomes hidden from screen readers when initialization
actually succeeds. This ensures the textarea remains visible and accessible to
users if the editor fails to initialize.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 18263a56-5090-463e-aaab-134c9a9b541f
📒 Files selected for processing (9)
frontend/user-profile-wysiwyg-editor.jsfrontend/wysiwyg-editor.jsstatic/css/v3/user-profile.cssstatic/css/v3/wysiwyg-editor.csstemplates/v3/includes/_avatar_v3.htmltemplates/v3/includes/_field_checkbox.htmltemplates/v3/includes/_field_dropdown.htmltemplates/v3/includes/_field_include.htmltemplates/v3/includes/_wysiwyg_editor.html
🚧 Files skipped from review as they are similar to previous changes (3)
- templates/v3/includes/_field_dropdown.html
- frontend/wysiwyg-editor.js
- frontend/user-profile-wysiwyg-editor.js
Comment on lines
+20
to
+33
| <div class="wysiwyg-v3" data-wysiwyg="v3" x-data="{ jsReady: false }" x-init=" | ||
| jsReady = true; | ||
| $nextTick(() => { | ||
| autoInit('{{textarea_id}}'); | ||
| })" | ||
| > | ||
| <textarea | ||
| id="{{ textarea_id }}" | ||
| name="{{ textarea_name }}" | ||
| class="w-full min-h-[200px]" | ||
| placeholder="" | ||
| x-show="!jsReady" | ||
| :aria-hidden="true" | ||
| {% if disabled_expr %}:disabled="{{ disabled_expr }}"{% endif %} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp/repo 2>/dev/null || cd /root/codebase 2>/dev/null || pwd
find . -name "_wysiwyg_editor.html" -o -name "*wysiwyg*" -type f | head -20Repository: boostorg/website-v2
Length of output: 369
🏁 Script executed:
git ls-files | grep -i wysiwyg | head -20Repository: boostorg/website-v2
Length of output: 336
🏁 Script executed:
cat -n templates/v3/includes/_wysiwyg_editor.html | head -50Repository: boostorg/website-v2
Length of output: 1965
🏁 Script executed:
cat -n static/js/v3/wysiwyg-editor.js | head -100Repository: boostorg/website-v2
Length of output: 50376
🏁 Script executed:
rg "autoInit\s*=" static/js/v3/ -A 5 -B 2Repository: boostorg/website-v2
Length of output: 25983
🏁 Script executed:
rg "autoInit" frontend/wysiwyg-editor.js -A 8 -B 2Repository: boostorg/website-v2
Length of output: 524
🏁 Script executed:
rg "const initWysiwyg" frontend/wysiwyg-editor.js -A 20 | head -50Repository: boostorg/website-v2
Length of output: 862
🏁 Script executed:
rg "window\.autoInit" static/js/v3/wysiwyg-editor.js -B 5 -A 5Repository: boostorg/website-v2
Length of output: 45
🏁 Script executed:
grep -n "x-" templates/v3/includes/_wysiwyg_editor.htmlRepository: boostorg/website-v2
Length of output: 323
🏁 Script executed:
rg "jsReady" templates/v3/includes/_wysiwyg_editor.htmlRepository: boostorg/website-v2
Length of output: 333
Move fallback textarea visibility and accessibility to depend on successful editor initialization.
Lines 20–32 set jsReady = true before autoInit() completes. If initialization fails or autoInit is unavailable, the fallback textarea becomes hidden (via x-show="!jsReady") and inaccessible to screen readers (:aria-hidden="true" is always set), leaving users with no functional control.
Set jsReady = true only after initialization succeeds, and bind aria-hidden to the jsReady state:
Proposed fix
- <div class="wysiwyg-v3" data-wysiwyg="v3" x-data="{ jsReady: false }" x-init="
- jsReady = true;
- $nextTick(() => {
- autoInit('{{textarea_id}}');
- })"
+ <div class="wysiwyg-v3" data-wysiwyg="v3" x-data="{ jsReady: false }" x-init="
+ $nextTick(() => {
+ if (typeof autoInit === 'function') {
+ autoInit('{{ textarea_id|escapejs }}');
+ jsReady = true;
+ }
+ })"
>
<textarea
id="{{ textarea_id }}"
name="{{ textarea_name }}"
class="w-full min-h-[200px]"
placeholder=""
x-show="!jsReady"
- :aria-hidden="true"
+ :aria-hidden="jsReady"🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 26-26: Special characters must be escaped : [ < ].
(spec-char-escape)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/v3/includes/_wysiwyg_editor.html` around lines 20 - 33, The
wysiwyg-v3 editor template sets jsReady to true immediately before autoInit()
completes asynchronously, causing the fallback textarea to be hidden via
x-show="!jsReady" even if initialization fails. Move the jsReady = true
assignment to execute after autoInit() completes successfully (likely within a
callback or promise chain inside the autoInit() function itself or in the x-init
block after awaiting it), and change the hardcoded :aria-hidden="true" attribute
on the textarea to :aria-hidden="jsReady" so the fallback textarea only becomes
hidden from screen readers when initialization actually succeeds. This ensures
the textarea remains visible and accessible to users if the editor fails to
initialize.
julhoang
requested changes
Jun 17, 2026
julhoang
left a comment
julhoang
left a comment
Collaborator
There was a problem hiding this comment.
Hi @jlchilders11, thank you for all the updates you've made! 🙌 I believe there's one remaining request here from your thread with @herzog0 . I believe resolving that should also clear up the current issue where the V3 demo page doesn't render the WYSIWYG correctly:
Additionally, I was curious if we could refactor wysiwyg-editor.js to support toolbar customization rather than duplicating the editor into a separate minimal-variant file. Making the toolbar configurable (e.g. a preset arg so this profile page just asks for a smaller set of tools) feels like a more maintainable long-term approach – it'd let us spin up new variants without forking the file each time. I pushed my proposed approach to this branch – I would love to hear what you think.
That said, I don't think this should hold up your PR considering we have several other tasks relying on it, so I'm definitely happy to leave it to a follow-up improvement task!
Collaborator
Author
Julia, thank you so much for that commit, that in fact was an improvement that Teo and I had been discussing as a future upgrade, that you just made happen! I'm happy to incorporate it into this PR, since it works for me. |
julhoang
approved these changes
Jun 19, 2026
julhoang
left a comment
julhoang
left a comment
Collaborator
There was a problem hiding this comment.
As discussed in the team chat, we'll leave the Avatar UI and functionality to a follow-up PR as we potentially have design adjustments pending. Other than that everything looks great to me. Thanks a ton for your awesome work on this ticket! 🙌
javiercoronadonarvaez
force-pushed
the
jc/user-edit-ui
branch
2 times, most recently
from
46daef2 to
54a52a7
Compare
…ost errors, remove duplicate delete button
…component, add styling to profile link header, add default checked state to field_include, change how avatar_v3 handles image sizing
jlchilders11
force-pushed
the
jc/user-edit-ui
branch
from
438a0ea to
ffd4e55
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue: #2437
Summary & Context
Adds an edit mode to the user profile for editing the individual components of the user profile.
NOTE: This includes no actual edit functionality, which will be added in several additional tickets.
Changes
v3/user_profile_edit.html, which is served when the appropriate query param is passedv3/includes/_field_include.htmlwhich accepts a django bound field and renders it using the appropriate include field template.field_dropdowncomponent with a disabled parameter and stylinguser/formsto capture the information from this page for processing.Please list any potential risks or areas that need extra attention during review/testing
Screenshots
Light Mode:
Dark Mode:
Mobile:
No Avatar Set:
Self-review Checklist
Frontend
Summary by CodeRabbit
xxxlavatar size, new underline button styling, and improved checkbox/dropdown styling (including optional field help text).