Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements BpkInputV2 with native HTML input elements instead of Chakra UI, removing all ChakraProvider dependencies. The implementation discovered that Chakra UI provided no benefits for simple input components, so the final solution uses native HTML with Backpack styling for better performance and simpler maintenance.
Changes:
- Replaced original Chakra UI approach with native HTML
<input>elements - Removed all ChakraProvider wrappers and Chakra UI dependencies from the component
- Enhanced BpkInputGroup with stricter TypeScript constraints (ReactElement instead of ReactNode for children)
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/001-bpkinput-chakra-ui/spec.md | Documents the final implementation decision to use native HTML instead of Chakra UI, with comprehensive rationale |
| specs/001-bpkinput-chakra-ui/tasks.md | Task breakdown showing completed migration and removal of ChakraProvider |
| specs/001-bpkinput-chakra-ui/styling-guide.md | SCSS styling patterns for BpkInputV2 and BpkInputGroup using Backpack tokens |
| specs/001-bpkinput-chakra-ui/research.md | Research findings documenting ChakraProvider availability and integration strategy |
| specs/001-bpkinput-chakra-ui/plan.md | Implementation plan template for component development |
| specs/001-bpkinput-chakra-ui/checklists/requirements.md | Specification quality validation checklist |
| specs/001-bpkinput-chakra-ui/api-design.md | API design for BpkInputV2 and BpkInputGroup components |
| packages/bpk-component-input/src/BpkInputV2/test-utils.tsx | Test utilities with ChakraProvider wrapper (now unused but retained) |
| packages/bpk-component-input/src/BpkInputV2/index.tsx | Export file without ChakraProvider wrapper |
| packages/bpk-component-input/src/BpkInputV2/common-types.ts | TypeScript type definitions for BpkInputV2 props |
| packages/bpk-component-input/src/BpkInputV2/accessibility-test.tsx | Accessibility tests using native render (no ChakraProvider) |
| packages/bpk-component-input/src/BpkInputV2/BpkInputV2.tsx | Main component implementation using native HTML input |
| packages/bpk-component-input/src/BpkInputV2/BpkInputV2.module.scss | Component styles using Backpack design tokens |
| packages/bpk-component-input/src/BpkInputV2/BpkInputV2-test.tsx | Unit tests for BpkInputV2 component |
| packages/bpk-component-input/src/BpkInputV2/BpkInputGroup/* | BpkInputGroup implementation and tests |
| examples/bpk-component-input-v2/* | Storybook examples for BpkInputV2 and BpkInputGroup |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,655 @@ | |||
| # Component Specification: BpkInputV2 Migration (Chakra UI → Native HTML) | |||
There was a problem hiding this comment.
The title describes a migration from Chakra UI to Native HTML, but the PR title states '[AI-Component]reimplement BpkInput' suggesting this is a new implementation. Consider clarifying whether this is a reimplementation or a migration, and ensure the title matches the actual work.
| # Component Specification: BpkInputV2 Migration (Chakra UI → Native HTML) | |
| # Component Specification: BpkInputV2 Reimplementation via Migration (Chakra UI → Native HTML) |
|
|
||
| import type { ReactElement } from 'react'; | ||
|
|
||
| import { ChakraProvider, defaultSystem } from '@chakra-ui/react'; |
There was a problem hiding this comment.
The test-utils.tsx file imports and uses ChakraProvider, but according to the spec and index.tsx, ChakraProvider has been removed from the implementation. This test utility appears to be unused based on the final decision to use native HTML. Consider removing this file or updating documentation to clarify its purpose.
| large, | ||
| name, | ||
| onClear, | ||
| size, // Extract from rest to avoid conflict with Chakra's size prop |
There was a problem hiding this comment.
The comment references avoiding conflict with Chakra's size prop, but the component no longer uses Chakra UI. This comment should be removed or updated to reflect the current implementation.
| size, // Extract from rest to avoid conflict with Chakra's size prop | |
| size, |
| ); | ||
|
|
||
| export const InvalidExample = () => ( | ||
| // <WithChakraProvider> |
There was a problem hiding this comment.
Commented-out ChakraProvider wrapper references remain in the code. Since ChakraProvider is no longer used, these comments should be removed.
| valid={false} | ||
| /> | ||
| </form> | ||
| // </WithChakraProvider> |
There was a problem hiding this comment.
Commented-out ChakraProvider wrapper references remain in the code. Since ChakraProvider is no longer used, these comments should be removed.
| @include forms.bpk-input; // Inherit all Backpack input styles | ||
|
|
||
| // Override Chakra Input defaults | ||
| &[data-chakra-input] { |
There was a problem hiding this comment.
The styling guide contains extensive references to Chakra UI data attributes and override strategies, but the final implementation uses native HTML without Chakra. This documentation is misleading and should be updated to reflect the actual implementation approach.
|
Visit https://backpack.github.io/storybook-prs/4166 to see this build running in a browser. |
Remember to include the following changes:
[Clover-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md