-
Notifications
You must be signed in to change notification settings - Fork 8
Fix create-publisher modal background to use dark theme #178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-authored-by: snomiao <[email protected]>
Co-authored-by: snomiao <[email protected]>
🎨 Chromatic Visual Testing Results
Check the visual changes and approve or request changes as needed. |
🎨 Chromatic Visual Testing Results
Check the visual changes and approve or request changes as needed. |
🎨 Chromatic Visual Testing Results
Check the visual changes and approve or request changes as needed. |
🎨 Chromatic Visual Testing Results
Check the visual changes and approve or request changes as needed. |
- Change import from @storybook/react to @storybook/nextjs-vite - Resolves linting error about direct renderer package imports
- Change openModal to open to match component props - Fixes TypeScript build error
- Set HUSKY=0 environment variable to prevent pre-commit hooks - Fixes lint-staged empty commit error in CI
- Create new reusable CreatePublisherFormContent component - Contains form logic and UI for creating a publisher - Accepts onSuccess and onCancel callbacks for flexibility
- Remove duplicate form logic and state management - Use extracted CreatePublisherFormContent component - Simplify modal to only handle modal-specific concerns - Fixes duplicate title issue
- Remove duplicate form logic and state management - Use extracted CreatePublisherFormContent component - Simplify page to only handle page-specific concerns (breadcrumb, routing)
Resolved conflicts: - .github/workflows/react-ci.yml: Keep both condition check and HUSKY env var - .storybook/main.ts: Adopt new organized stories structure from main - Migrate CreatePublisherModal.stories.tsx to new components/ directory structure
Update import to use relative path since story file is now co-located with the component in components/publisher/ directory after Storybook restructure.
- Create CreatePublisherPageLayout component for Storybook - Demonstrates the full create publisher page UI - Located at components/pages/create.stories.tsx as requested - Includes breadcrumb navigation and form content Addresses review comment on PR #178
|
Done! Added Storybook story at |
- Add showTitle prop to CreatePublisherFormContent component - Display "Create Publisher" heading when showTitle is enabled - Enable title display on the create publisher page - Update Storybook story to show title Addresses review comment requesting addition of page title. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
✅ Review comments addressed! Added the "Create Publisher" title heading to the create publisher page as requested:
The title is now displayed prominently at the top of the form on the create publisher page, while the modal version continues to use its own modal header. All CI/CD checks are passing! ✅ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes the create-publisher modal background to use the dark theme (bg-gray-800 / rgb(31 41 55)) instead of the white background that was previously displayed. The fix also refactors the form logic into a shared component for better reusability.
Key changes:
- Updated
customThemeTModaltheme configuration to usebg-gray-800for modalinnerandbody.baseclasses - Extracted form logic into a new
CreatePublisherFormContentcomponent to be shared between modal and page views - Added Storybook stories for visual testing of the modal and page components
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/comfyTheme.tsx | Fixed modal theme configuration to use dark background colors |
| components/publisher/CreatePublisherModal.tsx | Refactored to use the new shared form component |
| pages/publishers/create.tsx | Refactored to use the new shared form component |
| components/publisher/CreatePublisherFormContent.tsx | New shared component containing the publisher creation form logic |
| components/publisher/CreatePublisherModal.stories.tsx | Storybook story for testing the modal component |
| components/pages/create.stories.tsx | Storybook story for testing the page layout |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /> | ||
| </div> | ||
|
|
||
| <div className="flex center gap-4"> |
Copilot
AI
Oct 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The className 'center' is not a valid Tailwind CSS utility class. This should be 'justify-center' or 'items-center' depending on the intended alignment. Based on the context and similar patterns in the codebase, this should likely be 'justify-center'.
| <div className="flex center gap-4"> | |
| <div className="flex justify-center gap-4"> |
| {!isLoadingValidation && | ||
| publisherValidationError && ( | ||
| <> | ||
| <span className="font-medium"></span>{' '} |
Copilot
AI
Oct 25, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This empty span with only a className serves no purpose and should be removed. The space character following it can be achieved through other means if needed.
| <span className="font-medium"></span>{' '} | |
| {' '} |
The create-publisher modal was displaying with a white background instead of the expected dark gray (
rgb(31 41 55)/bg-gray-800), creating a visual inconsistency in the dark theme.Root Cause
The issue was in
utils/comfyTheme.tsxwhere thecustomThemeTModaltheme configuration had:innerclass usingbg-white bg-gray-200instead of dark backgroundbody.baseclass usingbg-gray-100instead of dark backgroundChanges Made
innerclass frombg-white bg-gray-200tobg-gray-800body.baseclass frombg-gray-100tobg-gray-800Impact
This fix affects 20 modal components throughout the application that use the
customThemeTModaltheme, ensuring consistent dark theme styling across:The background now properly uses the dark gray color (
rgb(31 41 55)) that matches the overall application theme.Fixes #147.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.