[NO-JIRA][BpkCloseButton&BpkSplitInput] Migrate entry point from js to ts#4316
[NO-JIRA][BpkCloseButton&BpkSplitInput] Migrate entry point from js to ts#4316
Conversation
There was a problem hiding this comment.
Pull request overview
Migrates the public entry points for bpk-component-split-input and bpk-component-close-button from Flow-annotated JS to TypeScript, aligning with the repo’s TypeScript usage while preserving the existing public API exports.
Changes:
- Replaced
index.jsentry points withindex.tsand removed stale/* @flow strict */annotations. - Exported
PropsfromBpkSplitInput.tsxso it can be re-exported asBpkSplitInputPropsvia the package entry point.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/bpk-component-split-input/src/BpkSplitInput.tsx | Exports Props interface to enable type re-export from the entry point. |
| packages/bpk-component-split-input/index.ts | TypeScript entry point; removes Flow annotation while keeping the same exports (default, INPUT_TYPES, BpkSplitInputProps). |
| packages/bpk-component-close-button/index.ts | TypeScript entry point; removes Flow annotation while preserving default export. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Visit https://backpack.github.io/storybook-prs/4316 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/4316 to see this build running in a browser. |
|
Visit https://backpack.github.io/storybook-prs/4316 to see this build running in a browser. |
IrinaWei
left a comment
IrinaWei
left a comment
There was a problem hiding this comment.
Code review
Found 2 issues (reviewed by 4/6 agents — Agent 2 skipped: no .scss files; Agent 6 skipped: no relevant historical comments):
🤖 Generated with Claude Code
| <div className={getClassName('bpk-drawer__close-button')}> | ||
| <BpkCloseButton label={closeLabel} onClick={onClose} /> | ||
| <BpkCloseButton label={closeLabel ?? ''} onClick={onClose} /> | ||
| </div> |
There was a problem hiding this comment.
Empty aria-label on close button when closeLabel is omitted
BpkCloseButton uses label directly as both aria-label and title on the <button>. With closeLabel ?? '', when closeLabel is undefined the button renders aria-label="" — an explicit empty accessible name. This is a WCAG 2.2 SC 4.1.2 violation: screen readers will announce the button as unlabelled.
Fix: Make closeLabel required in BpkDrawerContent props when the close button is rendered, or guard against rendering the close button when the label is absent — do not fall back to "".
| <div className={getClassName('bpk-drawer__close-button')}> | ||
| <BpkCloseButton label={closeLabel} onClick={onClose} /> | ||
| <BpkCloseButton label={closeLabel ?? ''} onClick={onClose} /> | ||
| </div> |
There was a problem hiding this comment.
Snapshot will fail — needs regeneration
The closeLabel ?? '' change is a rendered-output change: when closeLabel is undefined, the button previously had no aria-label/title attributes; after this PR it renders aria-label="" title="". Tests that render without passing closeLabel (e.g. the padded=true test) will produce a different DOM and the existing .snap will not match.
Fix: After resolving the empty-string issue above, run npm test -- --updateSnapshot for bpk-component-drawer and commit the regenerated snapshot.
Description
Migrate
bpk-component-split-inputandbpk-component-close-buttonentry points from Flow-typed JS to TypeScript.Changes
bpk-component-split-input/index.js→index.ts: removed stale/* @flow strict */annotationbpk-component-split-input/src/BpkSplitInput.tsx: addedexporttointerface Propsso it can be re-exported viaindex.tsbpk-component-close-button/index.js→index.ts: removed stale/* @flow strict */annotationImpact
No breaking changes. The public API surface (
export default,export { INPUT_TYPES },export type { BpkSplitInputProps }) is identical to before. Consumers import from the compileddist/index.jswhich is unaffected by the source file rename.Checklist
Remember to include the following changes:
[Clover-123][BpkButton] Updating the colourREADME.md(If you have created a new component)README.md