-
Notifications
You must be signed in to change notification settings - Fork 24
fixed styles for job-rb #79
fixed styles for job-rb #79
Conversation
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.
@dashu-baba most issues are fixed, but some have issues, and some break something else after fixing. Also, there are places in code that I'd like to improve, please, see comments for the code.
-
Placeholder font is still different. Also, there should be no uppercase.
-
After this, the selected option in select is vertically aligned, but the arrow and cross now jump up. It should stay vertically aligned:
-
Before the fix, skills select shows all the skills. But after the select is shows only limited number of skills with no scrollbar:
-
During resource booking editing there is some white gap now:
-
Also, there are not aligned:
-
During editing Resource Booking for Client Rate and Member Rate labels are still not shown:
-
When I make the browser window smaller the form fields are jumping, see video https://monosnap.com/file/rYabiIf06iMVnOhx31DLIsmnrobRZh.
They should not jump, they only have to be max-width 640px. And as soon as window is smaller they just shrink together with the window.
src/components/FormField/index.jsx
Outdated
onBlur={(event) => { | ||
input.onBlur(event); | ||
}} | ||
onFocus={(event) => { | ||
input.onFocus(event); | ||
}} |
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.
Can we directly pass like onBlur={input.onBlur}
and onFocus={input.onFocus}
?
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.
-
Spacing around skils is still changing when new row of skills is added, see demo video https://monosnap.com/file/g3nsn77R7fwvIwMcVjwHMfL4CRCMGu
- when we have only 1 line of skills spacing is perfect
- when I add the 2nd line of skills, the spacing above the first line become small, but it should stay the same
- when I add the 3rd line of skills, the spacing between 1st and 2nd line changes, and it's different from the spacing between 2nd and 3rd line. But all of them should stay the same during adding and removing lines
-
Placeholder inside Select is different
-
[extra +$10] Select height is
42px
, but it should be40px
as all other fields.
Also, the border color of the Select is different from the provided design and other fields -
Not sure if I understand correctly, I made the form max size to 640px. So it will be like that until the container with reduce. It will then reduce the padding with smooth transition.
Expected behavior is like on this video https://monosnap.com/file/pw2BhvvCdFEVfKVPaeufM5HMyGYwzR or check the live page https://connect.topcoder-dev.com/projects/16957/scope
- Form should have
width: 100%
to make it fluid, but havemax-width: 640px
so it doesn't become too wide. And it should be centered. So we don't need any media conditions for different screen sizes. Only make width of the form fluid with max-width 640px and centered.
- Form should have
Also, there are few comments for the code below.
{ !field.readonly && ( | ||
<label | ||
styleName={ | ||
(input.value != "undefined" && input.value !== null && input.value !== "") || meta.active |
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.
It seems that this condition input.value != "undefined"
checks for the string "undefined"
not for the value undefined
. Btw, there is a lodash method _.isNil to check if value is undefined
or null
.
src/components/ReactSelect/index.jsx
Outdated
styles={customStyles} | ||
onChange={(val) => { | ||
onChange(val); | ||
props.onChange(val); |
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.
Can we replace this with onChange={props.onChange}
?
@media (max-width: 1366px) { | ||
.tc-form{ | ||
.job-form-fields-container { | ||
padding-left: 15%; | ||
padding-right: 15%; | ||
transition: 300ms; | ||
} | ||
} | ||
} | ||
|
||
@media (max-width: 1280px) { | ||
.tc-form{ | ||
.job-form-fields-container { | ||
padding-left: 10%; | ||
padding-right: 10%; | ||
transition: 300ms; | ||
} | ||
} | ||
} | ||
|
||
@media (max-width: 1020px) { | ||
.tc-form{ | ||
.job-form-fields-container { | ||
padding-left: 5%; | ||
padding-right: 5%; | ||
transition: 300ms; | ||
} | ||
} | ||
} | ||
|
||
@media (max-width: 854px) { | ||
.tc-form{ | ||
.job-form-fields-container { | ||
padding-left: 0; | ||
padding-right: 0; | ||
transition: 300ms; |
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 don't need media conditions for this functionality, everything we need is:
- make all the fuilds fluid so they take 100% width of the form
- make the form 100% width but with max-width 640px
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.
Thank you for the quick turn around in fixing all the small issues, @dashu-baba. All works good now.
No description provided.