Add props to pass id and name attributes to the <input> #41
Conversation
CONTRIBUTING.md
Outdated
| @@ -1,18 +1,22 @@ | |||
| # Contributing | |||
|
|
|||
| Thanks for your interest in contributing to `react-tailwindcss-datepicker`! Please take a moment to review this document **before submitting a pull request**. | |||
| Thanks for your interest in contributing to `react-tailwindcss-datepicker`! Please take a moment to | |||
There was a problem hiding this comment.
I was getting complaints from prettier until I reran the formatting. This was likely committed without running prettier?
package.json
Outdated
| @@ -65,5 +65,8 @@ | |||
| "tailwindcss": "^3.2.4", | |||
| "tslib": "^2.4.0", | |||
| "typescript": "^4.8.4" | |||
| }, | |||
| "publishConfig": { | |||
| "registry": "https://registry.npmjs.com" | |||
There was a problem hiding this comment.
I added this for those of us who have different default registries. I hope you don't mind. Let me know if you would like to remove it.
There was a problem hiding this comment.
I think it's better to leave this as it is. You can just add your default registry when you publish from your fork
| autoComplete="off" | ||
| role="presentation" |
There was a problem hiding this comment.
These are optional but should ensure other password managers leave these fields alone.
src/components/Input.tsx
Outdated
| onChange={handleInputChange} | ||
| /> | ||
|
|
||
| <button | ||
| type="button" | ||
| ref={buttonRef} | ||
| disabled={disabled} | ||
| className="absolute right-0 h-full px-3 text-gray-400 focus:outline-none disabled:opacity-40 disabled:cursor-not-allowed" | ||
| className="absolute top-0 right-0 h-full px-3 text-gray-400 focus:outline-none disabled:opacity-40 disabled:cursor-not-allowed" |
There was a problem hiding this comment.
Another minor fix I hope you don't mind me including. At least for me, the button was misaligned when rendering in my modal. Setting top-0 fixed the positioning in all my use-cases.
There was a problem hiding this comment.
I think it would be much better if we add toggleClassName prop to pass in Datepicker component, so you can override the toggle button's class.
There was a problem hiding this comment.
Sure. Do we want to just have it append classes or overwrite them completely?
There was a problem hiding this comment.
Let's try having it append at the end of toggle button's className
There was a problem hiding this comment.
Added toggleClassName to the button!
| @@ -68,7 +70,9 @@ const Datepicker: React.FC<Props> = ({ | |||
| readOnly = false, | |||
| minDate = null, | |||
| maxDate = null, | |||
| disabledDates = null | |||
| disabledDates = null, | |||
| inputId, | |||
There was a problem hiding this comment.
I think we should consider having the id and name be optional
There was a problem hiding this comment.
They are optional in the type. They just don't accept null. id and name do not accept null as a value on an input field so this better represents the actual type where the values are used.
|
Not sure why it removed the review request from @onesine but anything else you need me to do to get this merged? |
| @@ -230,15 +232,15 @@ export default function Playground() { | |||
| /> | |||
| </div> | |||
| <div className="mb-2"> | |||
| <label className="block" htmlFor="startWeekOnClassName"> | |||
| Start Week On | |||
There was a problem hiding this comment.
Moved "Start Week On" under the newly added "Toggle Class" input so the formatting of the inputs here looks a little better.
For whatever reason, 1password was convinced that it should be trying to suggest something in the date picker field. This was frustrating because the calendar button would overlap with the 1password button and cause 1password to pop open whenever clicking to open the calendar.
This PR adds two props to let us pass a custom id/name attribute to the date input field. Setting any custom name will stop 1password from trying to suggest inputs.