Add date/calendar module template

1. +++ b/templates/date_views/date-views-pager.tpl.php
   

   The folder beneath templates should just be date (not date_view) since that is the project name this module is bundled with.

2. +++ b/templates/date_views/date-views-pager.tpl.php
   @@ -0,0 +1,64 @@
   + * @file
   + * Template file for the bootstrap theme display.
   

   Incorrect/inadequate description of what this template file is for.

3. +++ b/templates/date_views/date-views-pager.tpl.php
   @@ -0,0 +1,64 @@
   + * Variables available:
   + *
   + * $plugin: The pager plugin object. This contains the view.
   

   These variables should follow existing core standards: https://www.drupal.org/docs/develop/coding-standards/api-documentation-a...

   It's also a couple missing variables being used in the template below that aren't described here: $pager_prefix (no suffix?) and $extra_classes.

4. +++ b/templates/date_views/date-views-pager.tpl.php
   @@ -0,0 +1,64 @@
   + */
   +?>
   

   Missing @ingroup templates at the bottom of the file's doxygen comment. Despite what the above link says, this is necessary for http://drupal-bootstrap.org to pick up and categorize this file properly.

5. +++ b/templates/date_views/date-views-pager.tpl.php
   @@ -0,0 +1,64 @@
   +    <div class="col-xs-12 col-md-4 col-md-offset-4">
   

   Why is this using col-md-offset-4? Please tell me this isn't to "align" the text in the center.

   I'm assuming this is the month? If so, it should just be using the text-center class.

6. +++ b/templates/date_views/date-views-pager.tpl.php
   @@ -0,0 +1,64 @@
   +        <h3><?php print $nav_title ?></h3>
   

   Is $nav_title already sanitized?

7. +++ b/templates/date_views/date-views-pager.tpl.php
   @@ -0,0 +1,64 @@
   +    <div class="col-xs-12 col-md-4">
   

   Should be a <nav> element since there's a <ul> element immediately inside it.

   This should also be before the header so it can be floated to the right properly on larger screens.

8. +++ b/templates/date_views/date-views-pager.tpl.php
   @@ -0,0 +1,64 @@
   +          <?php
   +          $text = '&laquo;';
   +          $text .= $mini ? '' : ' ' . t('Prev', array(), array('context' => 'date_nav'));
   +          $aria_type = $mini ? 'aria-label' : 'aria-labelledby';
   +          $prev_options['attributes'][$aria_type] = t('Prev', array(), array('context' => 'date_nav'));
   +          print l(t($text), $prev_url, $prev_options);
   +          ?>
   ...
   +          <?php
   +          $next_options['attributes'][$aria_type] = t('Next', array(), array('context' => 'date_nav'));
   +          print l(($mini ? '' : t('Next', array(), array('context' => 'date_nav')) . ' ') . '&raquo;', $next_url, $next_options);
   +          ?>
   

   Um. No.

   All this should be done in a preprocess function, creating new variables if necessary (to compensate for module's laziness of proper theming I'm assuming).

   Furthermore, it should be using _bootstrap_icon() with something like menu-left instead of « and menu-right instead of ».

   Finally, we shouldn't be passing a variable to t() just to pass to l(), especially since it's already been translated above that.

I already did.

Never mind... I was thinking of something else.

The above patch is not up to par with the quality of code that is currently in this base theme.

It will not be committed as is.

Like many other templates, complex variables (display logic), is done in preprocess functions.

Please follow the existing workflow and structure of this base theme.

This patch needs no work.

You're not the maintainer of this project. I outlined what needed to be fixed above and you ignored what you deemed "didn't make sense".

That's not how contrib works and certainly not how I work.

No I'm not "making up rules just for you". The code you reference already exists (long before I made this project into what it is). Would I like to change it, sure, but I can't. That is code that has been out for some time and cannot be changed because it'd introduce a BC break.

This patch is introducing new code. Code that should, regardless of where it came from, utilize a preprocess function for all that PHP logic (which is mostly what I was referring to).

Furthermore, you seem to keep forgetting that 7.x-3.x isn't HEAD; 8.x-3.x is and it's using <nav>:

http://drupal-bootstrap.org/api/bootstrap/templates%21system%21pager.htm...

Regarding the icons, no, neither 8.x-3.x or 7.x-3.x are using them, specifically for the pager, but that's really not the point. This isn't a pager. It's a different theme hook/template all together. One that you're making look like a pager, nothing more.

If I could go back and put icons in the pagers, I would. It just makes it look sharper and also helps with accessibility.

---

I was attempting to give you the benefit of the doubt since you at least provided a patch.

I was originally tempted to mark this as "Closed (won't fix)" solely for the following reasons:

1. The fact that this is against 7.x-3.x
2. The fact that no one, even remotely, has mentioned needing a template for this from the base theme.
3. This is a template for a contrib module that not everyone will have (despite it appearing to be "popular").

However, since you already provided a patch and this is just an addition, I gave feedback.

You implemented some changes and ignored some feedback entirely.

I'm really not an unreasonable guy, but if this is how you're going to be then I have no qualms about rejecting this issue based solely on your attitude and the above reasons.

This is a pager. 100%. You may like to install calendar module and take a look into the view. It pice of code is named pager everywhere in views/date/calendar code.

No, they're just navigation links, so probably ~16% a "pager". A true pager (historically) also has numbers indicating the actual page you are currently on and subsequent pages you can navigate to before and after.

Just because they use the term "pager" through out the code doesn't actually mean that it's a "pager". They could easily, just as well, be just simple inline links.

My attitude is to fix issues that affect me and others.

No, your attitude has been to undermine and berate this issue queue whenever something you don't find appealing to your satisfaction. It's actually been quite a rude experience.

---

Now, to actually get back to the code. I was going to add another lengthy list of reviews, but I figured it's just be easier to code it so you can see what I mean.

Note: this does require the latest dev as the following was just backported to 7.x-3.x #2840791: Add icon support to "link" theme hook.

1. I could have sworn that it falls back to a non-context specific translation if a specific context translation doesn't exist. However, I just re-read locale() and it's not that fancy. Sigh... "Prev" isn't an English word, it's an abbreviation for "Previous". I also looked up the translations for this context and you're right, it's all for "Prev"... I'll revert the string, sad.
2. Mini pagers are supported, they only show the icon. See http://drupal-bootstrap.org/api/bootstrap/templates%21date%21date-views-...

I created this related follow-up.