Form input component #112
Form input component #112
Conversation
… and function converter from px to rem units
…focus visible pseudoclass behaviour
PixelSculptor
requested review from
martao02p,
marcol13,
A-Wodnicki and
Arashayo
| @@ -0,0 +1,92 @@ | |||
| @use 'functions' as utils; | |||
There was a problem hiding this comment.
Great job with these mixins and functions 💪
| @@ -1,10 +1,12 @@ | |||
|
|
|||
| // Do not edit directly | |||
There was a problem hiding this comment.
That is good idea to group these variables, but I am afraid that when we add or modify one of variables in Figma Tokens, these changes will be overwritten. Alternatively we can build our own solution to translate JSON representation of tokens to SCSS file 😄
There was a problem hiding this comment.
Or maybe there is possiblity to add some comments through Figma plugin before deploying changes to repo
| } | ||
| @use '../abstracts/' as *; | ||
|
|
||
| @import url('https://fonts.googleapis.com/css2?family=Poppins:ital,wght@0,100;0,200;0,300;0,400;0,500;0,600;0,700;0,800;0,900;1,100;1,200;1,300;1,400;1,500;1,600;1,700;1,800;1,900&display=swap'); |
There was a problem hiding this comment.
Do we need all this font weights? Maybe we could limit this to 3 or 4 weights? For example: 200, 400, 600, 700. What do you think about this?
There was a problem hiding this comment.
Yes I will short this URL ;)
| required, | ||
| darkTheme | ||
| }) => { | ||
| const themePicker = darkTheme ? 'input-fragment--dark' : 'input-fragment' |
There was a problem hiding this comment.
Nice approach to apply correct class based on theme. Since we don't have proper design in our Figma it is not mandatory to implement this, but it is a good reminder to think about this in next meeting, because there are several things to discuss.
The best way (in my opinion) to create themes in React app, is creating proper ThemeProvider. Here is one example of using this: https://webtips.dev/how-i-theme-my-react-app-with-sass#google_vignette . We can build something similar, but it should be separate task for it 😉
There was a problem hiding this comment.
We can discuss on our daily this general solution to avoid next time copy paste our own solution for every component
| <div className={classNames(styles[themePicker])}> | ||
| <label | ||
| htmlFor={id} | ||
| className={classNames(styles['input-fragment__label'])} |
There was a problem hiding this comment.
If you are using only one style, you don't need to use classNames function
There was a problem hiding this comment.
Sure thanks :D
| const placeholderText = inputType !== 'password' ? placeholder : null | ||
|
|
||
| return ( | ||
| <div className={classNames(styles[themePicker])}> |
There was a problem hiding this comment.
Here, in classNames function, you can add another argument which could be a prop passing from parent.
Sometimes (especially in forms) programmers might want to customize a specific input to their needs. In that situation they want to pass their own styles to this component. For that case you could add className prop and use it here:
| <div className={classNames(styles[themePicker])}> | |
| <div className={classNames(styles[themePicker], className)}> |
| 'password', | ||
| 'textArea', | ||
| 'submit', | ||
| 'checkbox', |
There was a problem hiding this comment.
In my opinion, you could get rid of some of these variants in FormInput component. textArea, submit, checkbox, date, file, radio, range, search and time probably would have different styles or functionality than the current component 😄
| @@ -0,0 +1,3 @@ | |||
| @function px-to-rem-converter($px-value, $base: 16px) { | |||
| @return ($px-value * 1px / $base * 1rem); | |||
There was a problem hiding this comment.
When I tried to run our application, I get this warning:
Maybe we should use there calc or math.div indeed.
This PR includes: