Skip to content

docs: Update documentation for controller PR #1779#43

Merged
kronosapiens merged 1 commit intomainfrom
docs-update-1750435043
Jul 29, 2025
Merged

docs: Update documentation for controller PR #1779#43
kronosapiens merged 1 commit intomainfrom
docs-update-1750435043

Conversation

@tarrencev
Copy link
Contributor

This PR updates the documentation to reflect changes made in cartridge-gg/controller#1779

**Original PR Details:**
- Title: chore: update @cartridge/ui
- Files changed: package.json

pnpm-lock.yaml
pnpm-workspace.yaml

Please review the documentation changes to ensure they accurately reflect the controller updates.

    Updates documentation to reflect changes made in:
    chore: update @cartridge/ui

    Related controller PR: cartridge-gg/controller#1779
@vercel
Copy link

vercel bot commented Jun 20, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
docs ✅ Ready (Inspect) Visit Preview Jun 20, 2025 3:58pm

@claude
Copy link
Contributor

claude bot commented Jun 20, 2025

Claude finished @tarrencev's task —— View job


PR Review Complete

Todo List:

  • Read the modified file to understand the changes
  • Analyze changes in context of @cartridge/ui update
  • Provide comprehensive review feedback
  • Check for documentation quality and accuracy

Changes Summary:

The PR updates the import statement in src/pages/controller/examples/react.md:122:

- import { Button } from '@cartridge/ui-next'
+ import { Button } from '@cartridge/ui'

Review Feedback

Positive Aspects:

  • Correct Update: The change properly reflects the UI library consolidation from @cartridge/ui-next to @cartridge/ui
  • Minimal & Focused: Only one line changed, which is appropriate for a dependency update
  • No Orphaned References: Confirmed no other references to @cartridge/ui-next exist in the codebase
  • Consistent Naming: The new import follows standard naming conventions

⚠️ Critical Issue - Missing Dependency:

Location: src/pages/controller/examples/react.md:122

The documentation imports Button from @cartridge/ui, but this package is not included in the installation instructions (lines 14-32). Users following this guide will encounter a runtime error when trying to import the Button component.

Recommendation: Add @cartridge/ui to the installation commands:

npm install @cartridge/connector @cartridge/controller @cartridge/ui @starknet-react/core @starknet-react/chains starknet

📋 Additional Observations:

  • Code Quality: The code example follows React best practices with proper TypeScript typing
  • Security: No security concerns identified - standard UI component import
  • Performance: No performance implications from this change
  • Test Coverage: This is documentation-only change, no tests affected

🔧 Suggested Actions:

  1. High Priority: Update installation instructions to include @cartridge/ui dependency
  2. Optional: Consider adding a note about the library migration from @cartridge/ui-next to @cartridge/ui for users who might have older versions

Summary:

While the import change is correct and reflects the UI library update, the documentation will not work as written without adding @cartridge/ui to the installation instructions. This should be fixed before merging.


@kronosapiens kronosapiens merged commit aa49dc7 into main Jul 29, 2025
2 checks passed
@kronosapiens kronosapiens deleted the docs-update-1750435043 branch July 29, 2025 17:03
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