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

SOE-3584: Adding print--html.tpl.php for the print 2.2 upgrade #7

Merged
merged 2 commits into from
Jan 18, 2019

Conversation

minorwm
Copy link
Contributor

@minorwm minorwm commented Jan 17, 2019

READY FOR REVIEW

Summary

  • This fixes the print friendly pages by adding a tpl file.

Needed By (Date)

  • 01/17/2019

Urgency

  • 6/10

Steps to Test

  1. Checkout out this branch.
  2. Upgrade the contrib print module to 2.2: https://www.drupal.org/project/print
  3. drush cc all
  4. Make sure the page looks print friendly: print/magazine/article/anne-kiremidjian-cities-built-endure-disaster

Associated Issues and/or People

See Also

@@ -137,6 +137,10 @@ function stanford_soe_regions_theme() {
'template' => 'templates/digital-magazine-page',
'render element' => 'page'
),
'print__html' => array(
Copy link
Member

Choose a reason for hiding this comment

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

This solution looks good but I have a couple of questions to understand the issue and resolution path.

Why have you added this theme hook here? Where did it go in print 2.2? Was this ever an option?
What I want to know is why it worked before and why you chose to fix this issue by adding it to this module.

Copy link
Contributor Author

@minorwm minorwm Jan 17, 2019

Choose a reason for hiding this comment

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

@sherakama I added it there because I didn't want to put it in the theme and the regions module is where the digital magazine template is located, so it seemed proper.

As far as why it worked before, I'm not 100% sure on that, but it had something to do with this: https://www.drupal.org/node/2276407

I figured I could get it working in 2.2 and then get a template to load correctly.

Copy link
Member

@sherakama sherakama left a comment

Choose a reason for hiding this comment

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

Please comment on the question. The code looks fine.

@sherakama sherakama merged commit 42dba63 into 7.x-1.x Jan 18, 2019
@sherakama sherakama deleted the SOE-3584 branch January 18, 2019 16:20
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.

2 participants