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
Pass id prop to DatePicker input field #8598
Conversation
Codecov Report
@@ Coverage Diff @@
## master #8598 +/- ##
=======================================
Coverage 85.25% 85.25%
=======================================
Files 195 195
Lines 4674 4674
Branches 1378 1378
=======================================
Hits 3985 3985
Misses 689 689
Continue to review full report at Codecov.
|
Could you update RangePicker at the same time? |
@yesmeck actually.. I just think this could work with P.S. we just need to make DatePicker work with onFocus |
6af3191
to
1cc7b87
Compare
To have the id on the initial input field would help us to write better dom selectors while testing: |
Conflict.... |
1cc7b87
to
2b1ac1e
Compare
conflicts are resolved, tests are running fine locally. I don't know why the snapshots of collapse are failing on the travis ci build. |
@mgrdevport Rebase master |
aa75654
to
ccd0317
Compare
@yesmeck Thanks |
@@ -267,6 +267,7 @@ export default class RangePicker extends React.Component<any, any> { | |||
disabled={props.disabled} | |||
readOnly | |||
value={(start && start.format(props.format)) || ''} | |||
id={props.id && `${props.id}-start`} |
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 should add id in the wrapper, instead inputs..
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.
That would mean we would have to search for the first and second input inside the wrapper:
findById(myid)->find(input)[nth]
What do you think of adding an additional id to the wrapper, without the start and end postfix?
findById(myid)->find(input#myid-start)
findById(myid)->find(input#myid-end)
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.
@yesmeck What your opinion. I will release a new version soon.
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.
Agree with @benjycui
Ping @mgrdevport |
ccd0317
to
0591aba
Compare
I've moved the id declaration to the wrapper. |
Conflict... Please rebase master again @mgrdevport |
0591aba
to
f99bb01
Compare
@ddcat1115 done. |
f99bb01
to
9e42be9
Compare
This PR passes the DatePicker prop id to the rendered input field as requested in #8305.