Convert event search to datepicker and cleanup template #12978
Conversation
|
(Standard links)
|
| <div class="crm-select-container">{$form.$elementName.html}</div> | ||
| <td class="{$campaignTdClass}"> | ||
| <label>{$form.$elementName.label}</label> | ||
| {$form.$elementName.html} |
There was a problem hiding this comment.
I feel good about this PR except for the changes to this file. This isn't directly related to events or dates, and i'm not sure about the change. Quickform already generates a <label> tag so I think this would result in <label><label>Label</label></label>
There was a problem hiding this comment.
@colemanw You are correct, we don't need the <label> tags here. The div is not required as the select element supplies it's own. And removing the <br /> means the elements takes up less vertical space on the screen - it also appears on contribution search and the change to single line helps I think. Happy to put this as a separate PR if you prefer?
mattwire
changed the title
mattwire
force-pushed
the
datepicker_manageevent_search
branch
from
a887738
to
7a02177
Compare
mattwire
changed the title
|
@colemanw Just need to you to confirm you are happy with the changes in response to your comments? |
|
Yes, although I'm confused by the screenshots -- there don't appear to be any datepicker elements in either screenshot? |
|
@colemanw Must be your eyes... I've updated the after screenshot to show the "Search ALL or by date range" selected at which point two datepickers magically appear :-) |
|
@agh1 Not directly related to this PR.. but any idea why I always get two "date" icons for datepickers like in the screenshot above? This was happening both before and after the date icon was changed. |
|
@mattwire that's weird--when I visit http://core-12978-ej5g.test-1.civicrm.org:8001/civicrm/event/manage?reset=1 I get this: |
|
Jenkins re-test this please. If the problem was introduced by #13003 it wouldn't be reflected in the test output because this test ran before that was merged. |
|
@mattwire I tried it with the new test build, and it looks correct: |
|
@agh1 Thanks for taking a look, must be something unique with my setup :-) |
|
@seamuslee001 @eileenmcnaughton So I think this is good to merge? |
|
merging based on @agh1 review & general concept approval for killing datepickers |
Overview
Convert form to use datepicker:
Partial from #12973
Before
Using old jcalendar.
After
Using "new" datepicker.
