thanks, we will look into this.
1. Same id attributes should be added to the tr elements in at least forms/event/location.php.
2. Another old grievance in these templates is that where.php has:
$required = '*';
location.php has:
$required = "<i>*</i>";
2.1. These values should be the same across all templates.
2.2. This should be an editable via a central setting so we don’t have to maintain custom templates just to set this to eg '& nbsp;*' (without the space).
As soon as these two are fixed we can cease the maintaining of several custom templates. Hope these will make it to EM 5.5!
3. Looking further into where.php I noticed that it is also the source of an issue I’ve been thinking of finding the cause of:
50 <?php if ( get_option ( 'dbem_gmap_is_active' ) ) : ?>
51 <div style="width: 400px; height: 300px; float:left;">
52 <div id='em-map-404' style='width: 400px; height:300px; vertical-align:middle; text-align: center;'>
53 <p><em><?php _e ( 'Location not found', 'dbem' ); ?></em></p>
54 </div>
55 <div id='em-map' style='width: 400px; height: 300px; display: none;'></div>
56 </div>
57 <?php endif; ?>
3.1. “Location not found” is not something we want to display to users upon loading a page. It should be displayed only by JS when appropriate. At the moment this can be disabled with only a custom template, or by some tricky CSS or JS, as this p element doesn’t have an id or class.
3.2. The whole section of code should be in a separate template or GUI setting. Location.php has a similar section; maybe others do too. They should all invoke the same code; change once, apply everywhere.
3.3. That code shouldn’t use direct style rules. To override these you need to use !important even for a general rule, and then to make exceptions to that rule it gets slimy.
3.4. Both classes and ids should be more used here. All the divs should have the same class, so you can change their attributes (eg. size) in one directive, but different ids, so you can set their floats etc. separately.
4. Every p element should also have an id and/or class. For example line 5 on where.php isn’t conveniently CSS-adjustable:
<p><?php _e("If you're using the Google Maps, the more detail you provide, the more accurate Google can be at finding your location. If your address isn't being found, please <a='http://maps.google.com'>try it on maps.google.com</a> by adding all the fields below seperated by commas.",'dbem')?></p>
These should all be good steps forward in code maintainability for both siteadmins and the EM dev team.
see the latest version of the plugin if you haven’t already, we’ve made some changes there:
http://wp-events-plugin.com/blog/2013/07/18/preview-of-events-manager-5-5/
I’ll take this into consideration for the next update and implement at least point 1 and 2. I am against IDs though (I always end up resorting to classes later in future), so it’d be classes only. In fact i know we need to remove one of those in location.php 🙂
The feedback is for the template in 5.4.4.2 (dev version of 5.5).
I don’t mind using individual classes instead of ids, there’s often not that much difference. Sometimes there is, though, as for example CSS selectors for ids can be used to override selectors for classes.
I thought it’d be a perfect time to fix these for the first stable in the 5.5 series. 5.5 is going to come with some changes to the templates, so siteadmins will be doing some manual updating in the hope that it would lead to less of that work in the future.
While we’re at it, it’s a great opportunity to make it count for even more – no more need for having a custom template to change the text for $required or to hide country or state rows with just a tiny bit more manual work when you’re doing manual work in any case.
will add what I can.
btw, in all cases you should always be able to override our CSS, precede the same rule we use with a wrapper class/ID unique to your theme, usually
body .em-class .etc { … }
will already be enough as it’s more specific than our one
Thanks for looking into this. We’d prefer to get as many template changes done with the 5.5 update as possible to save time in the long run.
Reviewed http://plugins.trac.wordpress.org/browser/events-manager/trunk/templates/forms/location/where.php and http://plugins.trac.wordpress.org/browser/events-manager/trunk/templates/forms/event/location.php of 5.4.4.3, the 2. beta of 5.5:
1. Fixed. Great!
2. Fix/workaround by keeping the setting static, but adding a filter for that. Nice! Hopefully affects also any other templates where $required is used.
3.1. Not fixed. http://plugins.trac.wordpress.org/browser/events-manager/trunk/templates/forms/map-container.php still opens the page stating “Location not found.” before the user has had time to submit any information on the location, which doesn’t make much sense. That string shouldn’t be shown by default, only when the loading of a map fails. Visible at http://demo.wp-events-plugin.com/submit-your-event/
3.2. Fixed. Great!
3.3. Fixed. Great!
3.4. Partially fixed. The only remaining <p> in where.php has a class, but the one in map-container.php doesn’t (having one could allow a JS workaround of 3.1). Also the only <p> in location.php is still missing a class, so “This event does not have a physical location.” can’t be conveniently styled in CSS only.
4. Fixed. Great!
3.1 – that’s for another release as I have other ideas there
3.4 will give the surrounding div class em-location-data-nolocation
