Skip to content

feat: admonition standardisation#2986

Closed
manika-signoz wants to merge 7 commits intomainfrom
feat/admonition
Closed

feat: admonition standardisation#2986
manika-signoz wants to merge 7 commits intomainfrom
feat/admonition

Conversation

@manika-signoz
Copy link
Copy Markdown
Contributor

@manika-signoz manika-signoz commented Mar 31, 2026

Summary

Closes https://github.com/SigNoz/engineering-pod/issues/3397
Rebuilds the docs Admonition component with SigNoz-themed styling, typed props, size variants (sm / lg), and a collapsible header (chevron toggle, optional defaultCollapsed). Removes the standalone KeyPointCallout component and maps the existing <KeyPointCallout /> MDX tag to Admonition for backwards compatibility.

Motivation / Problem

Current Admonitions did not align with designs.
Figma - https://www.figma.com/design/eyORbfrXMWCz9w0xEFdgWe/Periscope-%E2%80%93-Primitives-v2?node-id=12-749&p=f&m=dev

Changes

  • Refactor components/Admonition/Admonition.tsx to a client component: theme map per kind (note, tip, warning, danger, info, important, default), CVA-based variants, cn/class-variance-authority, Tailwind not-prose and nested code/pre styling.
  • Collapsible behavior: header button toggles body visibility; defaultCollapsed supports boolean or string "true" / "false".
  • Delete components/KeyPointCallout/KeyPointCallout.tsx; in components/MDXComponents.tsx, register KeyPointCallout as a thin wrapper around Admonition with default title / type when omitted.

Description

This PR replaces the old admonition UI with a token-aligned, accessible callout card. KeyPointCallout is no longer a separate component file; MDX that still uses <KeyPointCallout> renders through Admonition so existing guides and OpenTelemetry docs do not need a bulk tag rename.

Type of Change

  • Docs – Changes to data/docs/**
  • Blog – Changes to data/blog/**
  • Site Code – Changes to app/**, components/**, hooks/**, utils/**, config, etc.
  • Redirects – Renamed/moved docs or updated next.config.js redirects
  • Dependencies / CI / Scripts
  • Other – Explain below

Impact

  • Documentation only
  • UI changes
  • Developer workflow
  • Performance impact
  • Breaking change

If breaking change, describe migration steps:

  • Not breaking for MDX usage: <KeyPointCallout> and <Admonition> remain valid MDX components; visual appearance and behavior (collapsible header) change.

Context & Screenshots

visuals only for all types
Screenshot 2026-03-31 at 21 39 55

Screenshot 2026-03-31 at 21 40 28

Before / After (if applicable)

/blog/celery-worker - default
Screenshot 2026-03-31 at 21 34 14

/blog/opentelemetry-demo/ - KeyPointCallout replaced
Screenshot 2026-03-31 at 21 37 03

/docs/huggingface-observability/ - Admonition important type
Screenshot 2026-03-31 at 21 41 44

/docs/huggingface-observability/ - KeyPointCallout default collapsed
Screenshot 2026-03-31 at 21 44 05

/docs/ai/signoz-mcp-server/ - Admonition warning type
Screenshot 2026-03-31 at 21 46 40

/docs/aws-monitoring/sqs/ - Admonition tip type
Screenshot 2026-03-31 at 21 50 00

Checklist

General

  • Branch is up to date with main
  • PR title follows conventional format (type: description)
  • Self-reviewed the code
  • Comments added for complex logic

For all changes

  • Built locally (yarn build) with no errors
  • Ran yarn lint and fixed any issues
  • Pre-commit hooks passed (or ran yarn check:doc-redirects / yarn check:docs-metadata if applicable)

For docs changes (data/docs/**)

  • Followed the docs author checklist in contributing/docs-authoring.md
  • Added/updated the page in constants/docsSideNav.ts if adding or moving a doc
  • Any added docs images use WebP format and live under public/img/docs/<topic>/

For blog changes

  • Followed the blog workflow in contributing/blog-workflow.md
  • Frontmatter includes title, date, author, tags (and canonicalUrl if applicable)
  • Images use WebP format and live under public/img/blog/<YYYY-MM>/

For site code changes

For renamed or moved docs

  • Followed the redirects and discovery section in contributing/docs-authoring.md
  • Added permanent redirect in next.config.js under async redirects()
  • Updated internal links and sidebar in constants/docsSideNav.ts
  • Ran yarn check:doc-redirects to verify

Testing

  • Tested locally
  • Edge cases considered
  • Existing functionality verified
  • New tests added (if applicable)

Note: Submit as Draft by default. Mark "Ready for review" when checks pass and content is ready.

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 31, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
signoz-web Error Error Mar 31, 2026 6:46pm

Request Review

@manika-signoz manika-signoz marked this pull request as ready for review March 31, 2026 16:22
@manika-signoz manika-signoz requested a review from a team as a code owner March 31, 2026 16:22
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 31, 2026

Claude finished @manika-signoz's task in 2m 5s —— View job


Review: Admonition Standardisation

  • Read changed files and gather context
  • Analyze component architecture and theme system
  • Check backwards compatibility (KeyPointCallout used in ~166 files)
  • Post inline findings
  • Write summary

Summary

The refactor is well-structured — typed props, CVA variants, theme map, and cn usage are all consistent with the existing codebase patterns (e.g. components/ui/Button.tsx). The KeyPointCallout backwards-compat wrapper in MDXComponents.tsx is the right call given 166 files still reference it.

Issues Found

# Severity File Finding
1 🔴 Bug MDXComponents.tsx:129-137 {...props} spread after explicit title/type overrides defaults with undefined when caller omits those props — inline comment
2 🟡 Design Admonition.tsx:274-291 Every admonition is now collapsible (button + chevron) even when defaultCollapsed is not set, changing UX for all existing admonitions site-wide — inline comment
3 🟡 Cleanup Admonition.tsx:3 Unused ChevronUp import — inline comment
4 🔵 Nit Admonition.tsx:170-173 sm and lg root variants both resolve to p-4, making the size distinction a no-op for the root container — inline comment

Additional Notes

  • note and info themes are identical (both signoz_robin-*). If intentional, a comment explaining why would help future contributors.
  • The 6 exported CVA variant functions (admonitionRootVariants, admonitionHeaderVariants, etc.) are only consumed internally. Consider keeping them unexported unless they're intended for use by other components.

Checklist items not yet verified in the PR

  • yarn build passes with no errors
  • yarn lint passes
  • Verified on Vercel preview that existing <KeyPointCallout> pages render correctly (e.g. /blog/opentelemetry-demo/, /docs/huggingface-observability/)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant