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

Convert RecurringEntity form to datepicker #12746

Merged

Conversation

mattwire
Copy link
Contributor

Overview

Convert "Repeating entity" form to datepicker. This form is used by the create/edit activity dialog and the repeating events dialog.

Before

Form elements not using datepicker.
repeat_predatepicker

After

Form elements using datepicker. Cleaned up form slightly so elements are slightly better aligned, help icons are shown in the right places.
Start date moves down specifically because the first element will automatically receive focus and trigger the datepicker to popup when the form is loaded which is not desirable. I think it also makes sense to be further down.
It was also possible to upset the disable/enable elements code previously so that has also been modified so that it now works correctly.

repeat_postdatepicker

Technical Details

Comments

@civibot
Copy link

civibot bot commented Aug 29, 2018

(Standard links)

@mlutfy
Copy link
Member

mlutfy commented Aug 30, 2018

jenkins, test this please

$("#repeat_absolute_date").prop('disabled', true);
$("#repeat_absolute_date").removeClass('required');
$('#start_action_offset').prop('disabled', true);
$("#start_action_offset").removeClass('required');
Copy link
Member

Choose a reason for hiding this comment

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

This could be a one-liner in jQuery:

$("#repeat_absolute_date, #start_action_offset").prop('disabled', true).removeClass('required');

}
else if ($('input[name=ends][value=1]').prop('checked')) {
$('#start_action_offset').prop('disabled', false);
$("#start_action_offset").addClass('required').focus();
Copy link
Member

Choose a reason for hiding this comment

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

These 2 lines could be 1.

});
if ($('input[name=ends][value=2]').prop('checked')) {
$("#repeat_absolute_date").prop('disabled', false);
$("#repeat_absolute_date").addClass('required').focus();
Copy link
Member

Choose a reason for hiding this comment

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

These 2 lines could be 1.

@mattwire mattwire force-pushed the repeat_converttodatepicker branch from eff30fc to aff91e5 Compare September 3, 2018 21:40
@mattwire
Copy link
Contributor Author

mattwire commented Sep 3, 2018

@colemanw Thanks, I've updated with your jquery improvements

@colemanw
Copy link
Member

colemanw commented Sep 4, 2018

@civicrm-builder retest this please

@colemanw
Copy link
Member

colemanw commented Sep 4, 2018

I tried creating and editing repeat activities with this PR and I agree it's an improvement :)

@eileenmcnaughton
Copy link
Contributor

test this please

@eileenmcnaughton
Copy link
Contributor

test fails unrelated

@eileenmcnaughton eileenmcnaughton merged commit 8e39301 into civicrm:master Sep 13, 2018
@mattwire mattwire deleted the repeat_converttodatepicker branch September 25, 2018 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants