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

CRM-19490 Add in a shortdate format setting and ensure that date fiel… #9253

Merged
merged 1 commit into from
Oct 14, 2016

Conversation

seamuslee001
Copy link
Contributor

…ds are localised appropriately on confirmation page

…ds are localised appropriately on confirmation page
@seamuslee001
Copy link
Contributor Author

@mlutfy @eileenmcnaughton I think this should solve it the problem it would be good to get some localistation testing done on this.

@monishdeb
Copy link
Member

I might be wrong but didn't we already have short date formats to choose in https://www.example.org/civicrm/admin/setting/date?reset=1 ?

@seamuslee001
Copy link
Contributor Author

@monishdeb the only one that is in a short format like m/d/y would be the financial batch format. The other date display formats looks like they are either Year, Month (name) or full Date.

I didn't feel like re-purposing financial batch format date.

@seamuslee001
Copy link
Contributor Author

In theory someone could have re-used the complete date formats to shorten but not sure

@seamuslee001
Copy link
Contributor Author

I'm happy to be directed to use one of the current date formats and forget my new format

@monishdeb
Copy link
Member

monishdeb commented Oct 14, 2016

I think if there is no requirement to have a separate shortdate format for date display in confirm n thankyou page then lets use the existing one and update the label to say Date Format: Financial Batch and Contribution page. OR else we move the separate field you added from localization setting page to Date setting page. But I am open to any suggestions !!

@mlutfy
Copy link
Member

mlutfy commented Oct 14, 2016

The date settings are kind of messy right now. The "Date Format: Financial Batch" shouldn't be on /admin/setting/date, I think it should on the field/usage-specific settings (financial batch is something very specific).. but that's a different issue (there are lots of issues with that UI, which goes back to CiviCRM 1.x). Just saying we should probably not re-purpose that setting.

Adding a "short date" format seems rather harmless. Just curious: why you feel this new format is needed? Why is this specific case different? I never use "long date formats", I always use %Y-%m-%d (f-yeah ISO-8601).

@eileenmcnaughton
Copy link
Contributor

screen shot 2016-10-14 at 5 26 37 pm

You know birth_date - the original focus of this issue - should listen to the specific setting...

@seamuslee001
Copy link
Contributor Author

The problem here is that the field is in a profile and when on the confirmation page its outputting in American format all the time in M/D/Y. The reason i thought i should add in a short date format for display purposes was that i wanted to keep the output in the short format so in aussie it could be D/M/Y in others M/D/Y so we only see numbers as we always have on the confirmation page. But it also gives end users the ability to set their own format e.g. what @mlutfy has suggested.

@eileenmcnaughton
Copy link
Contributor

Ok my feeling is people have had a few different ideas here but no real objections have emerged. @seamuslee001 if you still feel, based on feedback, this is right I'm happy to merge

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton i think its probably the safest path to proceed is adding this new setting. The actual form situation is pretty small and i tested this locally and works fine.

@eileenmcnaughton eileenmcnaughton merged commit b13f46a into civicrm:master Oct 14, 2016
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.

4 participants