Skip to content
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

Allow strings for disabled days time picker #152

Merged
merged 2 commits into from
Oct 8, 2019

Conversation

Hyperkid123
Copy link
Member

@Hyperkid123 Hyperkid123 commented Oct 8, 2019

closes #150

Changes

  • strings can be now used to define disabled days for date picker, any string can be used as long as its accepted by Date constructor
  • added alias today which is equal to new Date()

@Hyperkid123 Hyperkid123 requested review from skateman and rvsia October 8, 2019 11:41
Copy link
Contributor

@rvsia rvsia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@codecov-io
Copy link

codecov-io commented Oct 8, 2019

Codecov Report

Merging #152 into master will increase coverage by 0.08%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #152      +/-   ##
==========================================
+ Coverage    80.7%   80.78%   +0.08%     
==========================================
  Files          86       87       +1     
  Lines        1337     1348      +11     
  Branches      318      321       +3     
==========================================
+ Hits         1079     1089      +10     
- Misses        214      215       +1     
  Partials       44       44
Impacted Files Coverage Δ
...c/form-fields/date-time-picker/date-time-picker.js 9.37% <0%> (-0.31%) ⬇️
...mapper/src/form-fields/date-time-picker/helpers.js 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b664b8d...d7ec567. Read the comment docs.

@skateman
Copy link
Member

skateman commented Oct 8, 2019

I'm not able to build it without errors, and I haven't found any documentation about it. I'dl ike to yarn link it into my codebase.

@Hyperkid123
Copy link
Member Author

Hyperkid123 commented Oct 8, 2019

@skateman try rm -rf node_modules yarn.lock && yarn && yarn vendor && yarn build

Yank link is a but different you have to cd to the desired package and run yarn link from there

@skateman
Copy link
Member

skateman commented Oct 8, 2019

Wait, what? I have to run the build from outside the package? From the monorepo?

@Hyperkid123
Copy link
Member Author

Hyperkid123 commented Oct 8, 2019

You can do it either way. Build from root builds every package

@skateman
Copy link
Member

skateman commented Oct 8, 2019

Yeah, running it from the package was failing.

@Hyperkid123
Copy link
Member Author

Do you have error log?

@skateman
Copy link
Member

skateman commented Oct 8, 2019

ERROR in ./demo/index.js
Module not found: Error: Can't resolve '@data-driven-forms/react-form-renderer' in '/home/skateman/Repositories/ManageIQ/react-forms/packages/pf3-component-mapper/demo'
 @ ./demo/index.js 22:0-66 45:186-198

ERROR in ./demo/demo-schemas/wizard-schema.js
Module not found: Error: Can't resolve '@data-driven-forms/react-form-renderer' in '/home/skateman/Repositories/ManageIQ/react-forms/packages/pf3-component-mapper/demo/demo-schemas'
 @ ./demo/demo-schemas/wizard-schema.js 1:0-88 4:15-29 27:19-33 32:19-33 46:16-30 55:19-33 65:19-33 72:19-33
 @ ./demo/index.js

ERROR in ./demo/demo-schemas/sandbox.js
Module not found: Error: Can't resolve '@data-driven-forms/react-form-renderer' in '/home/skateman/Repositories/ManageIQ/react-forms/packages/pf3-component-mapper/demo/demo-schemas'
 @ ./demo/demo-schemas/sandbox.js 2:0-116 18:21-31 26:21-31 36:21-31 42:21-31 51:21-31 57:21-31 59:18-28 66:21-31 71:21-31 77:21-31 83:18-28 86:21-31 92:21-31 99:21-31 101:19-29 109:21-31 111:19-29 113:17-27 125:21-31 140:21-31 142:19-29 151:21-31 167:21-31 183:21-31 195:19-29 197:17-27 210:21-31 230:21-31 249:21-31 269:21-31 287:21-31 304:19-29 306:17-27 318:21-31 327:21-31 329:19-29 337:21-31 342:21-31 344:19-29 346:17-27 359:21-31 365:21-31 371:21-31 374:19-29 376:17-27 388:21-31 393:21-31 398:21-31 403:21-31 409:21-31 428:21-31 443:21-31 445:19-29 447:17-27 449:15-25
 @ ./demo/index.js

ERROR in ./src/form-fields/component-mapper.js
Module not found: Error: Can't resolve '@data-driven-forms/react-form-renderer' in '/home/skateman/Repositories/ManageIQ/react-forms/packages/pf3-component-mapper/src/form-fields'
 @ ./src/form-fields/component-mapper.js 6:0-72 12:53-67 12:117-131 12:189-203 12:261-275 12:327-341 12:387-401 12:442-456 12:495-509 12:566-580 14:29-43 16:29-43 16:91-105 16:148-162
 @ ./src/index.js
 @ ./demo/index.js

ERROR in ./src/form-fields/layout-components.js
Module not found: Error: Can't resolve '@data-driven-forms/react-form-renderer' in '/home/skateman/Repositories/ManageIQ/react-forms/packages/pf3-component-mapper/src/form-fields'
 @ ./src/form-fields/layout-components.js 13:0-74 80:71-87 80:146-162 80:215-231 80:298-314 80:368-384
 @ ./src/index.js
 @ ./demo/index.js

ERROR in ./src/form-fields/form-fields.js
Module not found: Error: Can't resolve '@data-driven-forms/react-form-renderer' in '/home/skateman/Repositories/ManageIQ/react-forms/packages/pf3-component-mapper/src/form-fields'
 @ ./src/form-fields/form-fields.js 16:0-72 47:77-91 53:45-59 61:45-59 65:45-59 73:45-59 84:45-59 94:45-59 158:77-91 158:155-169 158:240-254 158:329-343 158:416-430 158:499-513 158:578-592 182:34-48 182:56-70 182:81-95 182:114-128 182:145-159 182:172-186 182:195-209 191:19-33 197:19-33 203:19-33 209:19-33 215:19-33 226:23-37 235:19-33
 @ ./src/form-fields/component-mapper.js
 @ ./src/index.js
 @ ./demo/index.js

ERROR in ./src/form-fields/multiple-choice-list.js
Module not found: Error: Can't resolve '@data-driven-forms/react-form-renderer' in '/home/skateman/Repositories/ManageIQ/react-forms/packages/pf3-component-mapper/src/form-fields'
 @ ./src/form-fields/multiple-choice-list.js 18:0-75 27:14-31
 @ ./src/form-fields/form-fields.js
 @ ./src/form-fields/component-mapper.js
 @ ./src/index.js
 @ ./demo/index.js

@Hyperkid123
Copy link
Member Author

Hyperkid123 commented Oct 8, 2019

You have to run build @data-driven-forms/react-form-renderer package first. Packages in monorepo are linked locally by default so we can test everything regardless where we make any change.

@skateman
Copy link
Member

skateman commented Oct 8, 2019

Yeah, this should be probably documented somewhere.

@@ -405,9 +405,10 @@ const output = {
component: components.DATE_PICKER,
isClearable: true,
disabledDays: [
'today',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What this line actually means? Is it that you can't select today in the picker? Would it disable any date befor today if I write before: 'today' below?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using this component. There is a link in the componentApi docs for date time picker

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this doesn't really answer my question, but I tested it and it works:

                  disabledDays: [{
                    after: 'today'
                  }]

This doesn't allow me to select any date in the future and that's the thing I want the most from this PR 😉

Copy link
Member

@skateman skateman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Seal of Approval

@rvsia rvsia merged commit 9d2d1c8 into master Oct 8, 2019
@Hyperkid123 Hyperkid123 deleted the allow-strings-for-disabled-days-time-picker branch October 8, 2019 13:52
@skateman
Copy link
Member

skateman commented Oct 8, 2019

@Hyperkid123 if you merge something, is there always a new version pushed? Would it be possible to comment it here the way we do it in react-ui-components? It would be super helpful...

@Hyperkid123
Copy link
Member Author

@skateman depends on the commit message. We are using semantic release message format
If there is something that is in the correct format new version will be release.

react-ui-components does do the same but you don't even have to have special commit message

@skateman
Copy link
Member

skateman commented Oct 8, 2019

Yeah, but I'm missing these kind of comments, so I don't have to hunt down the version changes all the time.

@Hyperkid123
Copy link
Member Author

Ah that. Yeah i wanted to do that for a long time. I just don't know when. You can check it out if you want there is even a bot account i think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Minimum date setting for date/datetime picker
4 participants