Conversation
|
| constructor(props: TextInputProps) { | ||
| super(props) | ||
| this.state = { | ||
| hasFocus: false, |
There was a problem hiding this comment.
This is not needed, the focus utility calculates this automatically
matyasf
force-pushed
the
textinput_new_theme
branch
3 times, most recently
from
fae2586 to
bcbebb3
Compare
| small: 'sm', | ||
| medium: 'md', | ||
| large: 'xl' |
There was a problem hiding this comment.
@design is this mapping correct? This means that size=small TextInput should use sm sized Lucide icons, medium should use md,..
There was a problem hiding this comment.
The large textInput variant should use the lg icon size that is 24 px.
|
@matyasf If it's in an error/success state, the focus outline remains blue. |
|
@matyasf think the upgare guide should be updated too. Some of the tokens have been renamed or removed. |
matyasf
force-pushed
the
textinput_new_theme
branch
from
bcbebb3 to
92f27ce
Compare
Fixed now |
I've added the token changes to the upgrade guide |
adamlobler
left a comment
adamlobler
left a comment
There was a problem hiding this comment.
I just left some comments, we also need to update some token values on our side, I try to merge these changes asap to v12 and we can check to outcome here if you rebase them.
matyasf
force-pushed
the
textinput_new_theme
branch
2 times, most recently
from
cd8d311 to
6d1d317
Compare
I've changed it according to our new rules |
adamlobler
left a comment
adamlobler
left a comment
There was a problem hiding this comment.
If you update the large icon size, I think its done ✅
…r TextInput Also fix a missing dependency in Pill To test: - compare with the design specs in Figma for TextInput component (it does not need the arrows) - it should work as before Completes INSTUI-4814
matyasf
force-pushed
the
textinput_new_theme
branch
from
6d1d317 to
1987961
Compare
4 participants
Also fix a missing dependency in Pill
To test:
NumberInput)Completes INSTUI-4814