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

dev/core#1847 Fix datepicker to respect the searchDate offsets #17762

Merged
merged 1 commit into from
Jul 13, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Jul 6, 2020

Overview

This restores previous behaviour where search datepicker fields had date ranges bounded by the values configured on the date preferences screenn
config screen

Before

(with default range of 20 years offset in either direction)
before screen shot

After

(with default range of 20 years offset in either direction)
Screen Shot 2020-07-06 at 1 48 51 PM

Technical Details

The min and max year are from civicrm_preferences_date table

Comments

I have some misgivings about this since I suspect the rationale behind the search offsets was primarily about the clunky UI.

Having said that I think the datepicker UI is a bit clunky on this front too as it's not obvious you can choose earlier
dates. This does at least restore established behaviour.

https://lab.civicrm.org/dev/core/-/issues/1847

ping @colemanw @seamuslee001 @mattwire @yashodha

@civibot
Copy link

civibot bot commented Jul 6, 2020

(Standard links)

@civibot civibot bot added the 5.28 label Jul 6, 2020
@seamuslee001
Copy link
Contributor

@eileenmcnaughton 3 test failures relate
CRM_Contact_Form_Task_EmailCommonTest::testPostProcessWithSignature
CRM_Case_BAO_CaseTest::testCaseClosure
CRM_Activity_Form_ActivityTest::testInboundEmailDisplaysWithLinebreaks

*/
public function &add(
$type, $name, $label = '',
$attributes = '', $required = FALSE, $extra = NULL
$attributes = [], $required = FALSE, $extra = NULL
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@seamuslee001 I think this makes sense for the test fails... it just gets otherwise converted & it's pretty nasty defaulting an array to a string

@eileenmcnaughton
Copy link
Contributor Author

Added has-test since clearly hitting test fails!

I have some misgivings about this since I suspect the rationale behind the search offsets was primarily about the clunky UI.

Having said that I think the datepicker UI is a bit clunky on this front too as it's not obvious you can choose earlier
dates. This does at least restore established behaviour.

https://lab.civicrm.org/dev/core/-/issues/1847
@eileenmcnaughton
Copy link
Contributor Author

OK - this is probably more conservative - will it pass now?

@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.2)

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.

2 participants