Skip to content

- artefacts loading title fix that shows on slow networks#369

Merged
spashii merged 1 commit intomainfrom
artefact-loading-title-fix-dashboard
Nov 22, 2025
Merged

- artefacts loading title fix that shows on slow networks#369
spashii merged 1 commit intomainfrom
artefact-loading-title-fix-dashboard

Conversation

@ussaama
Copy link
Copy Markdown
Contributor

@ussaama ussaama commented Nov 22, 2025

Summary by CodeRabbit

  • New Features
    • Enhanced verified artefacts section with improved loading state visualization and skeleton placeholders
    • Reorganized artefact presentation using accordion interface for cleaner content organization and better accessibility

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Nov 22, 2025

Walkthrough

Refactors VerifiedArtefactsSection component to consolidate loading and empty-state handling into an early return with skeleton UI, then rewrites artefact rendering into a single Accordion-driven map path, eliminating nested conditional rendering blocks and simplifying state logic.

Changes

Cohort / File(s) Summary
Component state & rendering refactor
echo/frontend/src/components/conversation/VerifiedArtefactsSection.tsx
Introduces early loading state guard returning skeleton placeholder; simplifies empty-state condition to ignore isLoading; consolidates separate rendering paths into single Accordion.Item map pattern with Accordion.Control displaying key/approval date and Accordion.Panel rendering Markdown content.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

  • Verify skeleton placeholder renders correctly when isLoading is true
  • Confirm empty-state guard logic handles null/undefined artefacts consistently
  • Spot-check Accordion.Item structure (Control/Panel) matches expected UX pattern
  • Validate Markdown content still renders properly within new Panel wrapper

LGTM—clean consolidation. Single-file refactor with straightforward conditional collapse. No API surface changes means no cascade risks. Accordion pattern looks solid.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Title check ⚠️ Warning The title references a 'loading title fix' but the actual changes involve restructuring artefact rendering, loading states, and UI layout—not fixing a title display issue. Revise the title to accurately reflect the main change, such as 'Refactor VerifiedArtefactsSection with early loading state and simplified artefact rendering' or 'Improve artefacts section loading UX with skeleton placeholder and accordion layout'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch artefact-loading-title-fix-dashboard

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2535955 and 44aab98.

📒 Files selected for processing (1)
  • echo/frontend/src/components/conversation/VerifiedArtefactsSection.tsx (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ussaama
Repo: Dembrane/echo PR: 205
File: echo/frontend/src/lib/query.ts:1444-1506
Timestamp: 2025-07-10T12:48:20.683Z
Learning: ussaama prefers string concatenation over template literals for simple cases where readability is clearer, even when linting tools suggest template literals. Human readability takes precedence over strict linting rules in straightforward concatenation scenarios.
🧬 Code graph analysis (1)
echo/frontend/src/components/conversation/VerifiedArtefactsSection.tsx (1)
echo/frontend/src/components/common/Markdown.tsx (1)
  • Markdown (6-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: ci-check-server
🔇 Additional comments (3)
echo/frontend/src/components/conversation/VerifiedArtefactsSection.tsx (3)

42-49: LGTM - Clean loading state extraction.

Early return pattern is 🔥. Kills the title flash bug on slow networks. Skeleton UI ships good UX.


52-52: LGTM - Simplified guard logic.

Clean separation of concerns. Loading handled upstream, this just guards empty state. Solid.


72-99: LGTM - Stellar refactor.

Single-path rendering FTW. Accordion map is clean, defensive nullish checks ship, i18n on point. No nested conditional soup. This scales.


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 and usage tips.

@spashii spashii added this pull request to the merge queue Nov 22, 2025
Merged via the queue into main with commit aa79d55 Nov 22, 2025
11 checks passed
@spashii spashii deleted the artefact-loading-title-fix-dashboard branch November 22, 2025 19:14
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