-
Notifications
You must be signed in to change notification settings - Fork 68
PD-1669 Add size variants to text input #1072
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
Size Change: +339 B (0%) Total Size: 394 kB
ℹ️ View Unchanged
|
@@ -107,6 +117,8 @@ interface TextInputProps extends HTMLElementProps<'input', HTMLInputElement> { | |||
handleValidation?: (value: string) => void; | |||
|
|||
['aria-labelledby']?: string; | |||
|
|||
sizeVariant?: SizeVariantType; |
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.
I think we should just call this prop size
to keep in line with other components
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.
I tried to use size
but it looks like since we're using the HTMLInputElement
interface, on line 52, that already has a property of size
with a type of number
so I was getting a TS error. To avoid that I changed it to sizeVariant
.
describe('when the "sizeVariant" is "large"', () => { | ||
test('check if font-size is 18px', () => { | ||
const { label } = renderTextInput({ | ||
value: validEmail, | ||
sizeVariant: SizeVariant.Large, | ||
optional: true, | ||
...defaultProps, | ||
}); | ||
|
||
expect(label).toHaveStyle({ | ||
fontSize: '18px', | ||
}); | ||
}); | ||
}); | ||
|
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 works for now, but this type of test is better suited to something like Chromatic or Percy
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.
We could even consider using this package https://www.npmjs.com/package/jest-image-snapshot
* Update package.json * Create shiny-sloths-joke.md
* update text input to include size variants * prettier fixes * updated website component * update readme again * prettier fixes * added input text size * changeset update * changed size default to default * add padding * remove medium size, add baseFontSize * naming updates, prettier fixes * update error font size * PD-1694 Upgrade `lib` dep in expandable card (#1073) * Update package.json * Create shiny-sloths-joke.md * removed submodules and updated node version in readme (#1075) Co-authored-by: Adam Thompson <[email protected]>
✍️ Proposed changes
🎟 Jira ticket: PD-1669
🛠 Types of changes
✅ Checklist
yarn changeset
and documented my changes🧪 How to test changes
💬 Further comments