Skip to content

feat(services/markdown): Add support for direct markdown content and update tests#712

Merged
hpintos merged 1 commit into
masterfrom
feat/add-content-capability-to-markdown
Sep 30, 2025
Merged

feat(services/markdown): Add support for direct markdown content and update tests#712
hpintos merged 1 commit into
masterfrom
feat/add-content-capability-to-markdown

Conversation

@hpintos

@hpintos hpintos commented Sep 30, 2025

Copy link
Copy Markdown
Contributor

COMPONENTS/SERVICES/MARKDOWN

✅ Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (tech debt, code improvement, adding a new hook)
  • Add/update documentation or literals (choose one option only)
  • Add/update tests (e2e, integration or unit testing)
  • Performance improvement (fix or feature that improves performance)

📝 Description, Motivation and Context

Add support for direct markdown content for ServicesMarkdown component

@jordevo jordevo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏 LGTM! 🚀

@hpintos hpintos merged commit 23b52a9 into master Sep 30, 2025
5 checks passed
@hpintos hpintos deleted the feat/add-content-capability-to-markdown branch September 30, 2025 14:15
if (req.readyState === COMPLETED && req.status === STATUS_OK) {
setHtml(marked(req.responseText))
setLoaded(true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if/else currently covers two branches, but the component actually has three. Since the src prop is no longer required, there’s now a third case where neither src nor contentprops are provided.

What do you think about making the code more defensive and explicitly handling that scenario by returning null? That would be clearer than falling back to the empty string in the state.

if (!content && !src) {
  return null
}

if (content) {
  setHtml(marked(content))
  setLoaded(true)
  return
}

if (src) {
  const req = new window.XMLHttpRequest()
  req.open('GET', src, true)
  req.onload = () => {
    if (req.readyState === COMPLETED && req.status === STATUS_OK) {
      setHtml(marked(req.responseText))
      setLoaded(true)
    }
  }
  req.send(null)
}

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.

6 participants