Conversation
14004a0 to
835da99
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adjusts how story IDs and properties are transformed in Storybook, and aligns input component props and defaults across BaseInput and InputText.
- Convert camelCase story titles and args to snake_case and wrap the iframe for dynamic resizing
- Make
typeoptional in BaseInput and set its default in BaseInput component - Rename
readOnlyprop toreadonlyin InputText API and map it correctly to the underlying input
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/storybook/addons/framework-selector/FrameworkSelectorDecorator.tsx | Add camelCaseToSnakeCase, update getStoryId, wrap iframe for resizing |
| packages/components/src/internal/partials/BaseInput/BaseInput.types.ts | Change type in BaseInputVisibleProps to optional |
| packages/components/src/internal/partials/BaseInput/BaseInput.tsx | Remove type destructure, set default, and simplify class logic |
| packages/components/src/InputText/InputText.types.ts | Rename readOnly prop to readonly |
| packages/components/src/InputText/InputText.tsx | Update destructured prop to readonly and map to readOnly attribute |
Comments suppressed due to low confidence (2)
packages/components/src/InputText/InputText.types.ts:20
- Renaming the prop from 'readOnly' to 'readonly' diverges from React’s standard
readOnlyprop and may confuse consumers. Consider retainingreadOnlyor providing a clear deprecation path.
readonly?: boolean;
src/storybook/addons/framework-selector/FrameworkSelectorDecorator.tsx:24
- There are no unit tests covering the new
camelCaseToSnakeCaseandgetStoryIdfunctions. Adding tests would help catch edge cases in string transformations.
const camelCaseToSnakeCase = (str: string) => str
| return accumulator; | ||
| } | ||
|
|
||
| const properyFinalName = camelCaseToSnakeCase(propertyName); |
There was a problem hiding this comment.
Typo in variable name: 'properyFinalName' should be 'propertyFinalName' to avoid confusion.
Suggested change
| const properyFinalName = camelCaseToSnakeCase(propertyName); | |
| const propertyFinalName = camelCaseToSnakeCase(propertyName); |
albozek
approved these changes
Jun 26, 2025
tischsoic
approved these changes
Jun 27, 2025
dew326
approved these changes
Jun 27, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
For QA:
Documentation: