-
-
Notifications
You must be signed in to change notification settings - Fork 104
Recurrence additionaldates #1524
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
base: master
Are you sure you want to change the base?
Conversation
d007cd4 to
458618e
Compare
petschki
left a comment
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.
LGTM
| ributtonExtraClass: "", | ||
|
|
||
| // INPUT CONFIGURATION | ||
| hasAdditionalDates: false, |
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.
For me allowAdditionalDates woud make more sense, as its in line with the z3c.form widget property.
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.
I changed to hasAdditionalDates as we already have hasRepeatForeverButton. But is there already a z3c.form widget property which is named allowAdditionalDates? I thought that was introduced in plone/plone.formwidget.recurrence#66
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.
No there isn't. I just wanted to be them in sync.
|
btw. were the latest two commits on master regarding recurrence widget intended to be in this PR? https://github.com/plone/mockup/commits/master/ |
|
@petschki no, I intentionally left them out of this PR, otherwise reviewing would be hard. This autoformatting did not change anything in the logic, it was just prettifying the code. @petschki what is your opinion again on |
…eview, show a inline warning instead of a window alert.
458618e to
4193b9b
Compare
| yearly: "Yearly", | ||
| }, | ||
|
|
||
| error_load_occurrences: "Cannot load the occurrences preview.", |
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.
so the translated strings are all provided via the plone.formwidget.recurrence template markup right? This should be implemented there too.
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.
my personal opinion: I want to allow adding additional dates or not ... so |
feat(pat-recurrence): Allow to enable/disable the feature to add additional dates.
Ref:
plone/plone.formwidget.recurrence#66
plone/plone.formwidget.recurrence#49
plone/plone.formwidget.recurrence#48
Does two things:
fix(pat-recurrence): In an error case when loading the occurrences preview, show a inline warning instead of a window alert.
before:
after:
feat(pat-recurrence): Allow to enable/disable the feature to add additional dates.
Without (default):
With:
TBD
If your pull request closes an open issue, include the exact text below, immediately followed by the issue number. When your pull request gets merged, then that issue will close automatically.
Closes #