Skip to content

Markdown::escape() for mail text, and keep the React settings section after a save - #33

Merged
sauce-base merged 2 commits into
mainfrom
dev-2026-10-29
Oct 29, 2026
Merged

sauce-base merged 2 commits into
mainfrom
dev-2026-10-29

Conversation

@roble

@roble roble commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Saucebase\Core\Support\Markdown::escape()

Mail Markdown escapes HTML in {{ }}, but CommonMark still renders [text](https://…), images, reference links and emphasis. Text that one person types and another reads (a workspace name in a tenancy email, a roadmap title, a passkey name in a security alert) could put a working link into a trusted email. escape() backslash-escapes the Markdown control characters (\ * _ [ ] < > !`) and leaves ordinary text alone.

Str::inlineMarkdown() doesn't cover this: it still renders https links.

Tests: tests/Unit/Support/MarkdownTest.php passes link, image, reference-link and emphasis payloads through Illuminate\Mail\Markdown::parse and asserts no <a>, <img> or <em> comes out. A second test checks plain text comes back unchanged.

React settings modal no longer jumps sections after a save

To reproduce: open settings on one section, switch to another from the sidebar, and save something there (for example invite a member). The modal jumps back to the first section.

After the save, the redirect back makes Inertia rewrite the URL with the fragment the page first loaded with. It uses replaceState, so no hashchange fires. React's useSettingsModal took its section from window.location.hash, so it followed the stale fragment. Vue's composable reads the fragment it tracks itself, so Vue wasn't affected.

Fix: the React hook's useSyncExternalStore snapshot reads the tracked fragment, which only hashchange and the hook's own writes update, the same as Vue.

Tests: a new case rewrites the hash without hashchange and asserts the section stays. Two older cases changed the hash without the event a browser fires; they now fire it. Core Vitest: 100 pass. Also checked in the React app: invite from Members after opening on Profile, and it stays on Members.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Added Markdown text escaping for characters that could otherwise be interpreted as links, images, emphasis, or code formatting.
  • Bug Fixes
    • Settings navigation now updates the displayed section in response to hash changes and keeps the reported section consistent when a change is not announced through a navigation event.

@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Walkthrough

The pull request updates how the settings modal reads URL fragments and adds a Markdown escaping utility with tests.

Changes: Settings modal fragment tracking

Layer / File(s) Summary
Tracked fragment parsing and tests
resources/js/react/hooks/useSettingsModal.ts, resources/js/react/hooks/useSettingsModal.test.ts
The hook now parses the fragment value from its external store subscription. Tests trigger hash-change listeners and verify that Inertia navigation does not change the reported section after an unannounced fragment update.

Markdown escaping

Layer / File(s) Summary
Markdown escaping utility and tests
src/Support/Markdown.php, tests/Unit/Support/MarkdownTest.php
Markdown::escape escapes selected Markdown syntax characters and returns the input if replacement returns null. Tests cover markup payloads and ordinary text.

Priority: Not assessed

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: Low · up to 9711d

Some text may render as headings or lists, and a redirect can prevent selecting a settings section. These bounded issues should be addressed or accepted before release.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@sonarqubecloud

Copy link
Copy Markdown

@sauce-base
sauce-base merged commit f49b62d into main Oct 29, 2026
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants