Problem/Motivation

On node pages there is a node type class of the form page-node-type-[node-type] however this class is not present on node preview pages, leading to preview pages not having the same styles as normal node view pages.

Proposed resolution

Add the node type class to the node preview pages also.

CommentFileSizeAuthor
#7 drupal-node_type_preview_class-2736391-7-complete.patch1.67 KBLoMo
#7 drupal-node_type_preview_class-2736391-7-tests.patch779 bytesLoMo
#6 drupal-node_type_preview_class-2736391-6-complete.patch1.67 KBLoMo
#6 drupal-node_type_preview_class-2736391-6-tests.patch778 bytesLoMo
#2 drupal-node_type_preview_class-2736391-2.patch936 bytesrooby
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rooby created an issue. See original summary.

rooby’s picture

rooby CreditAttribution: rooby commented
Status: Active » Needs review
FileSize
drupal-node_type_preview_class-2736391-2.patch936 bytes

Here is a patch to add the class.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir at MD Systems GmbH commented
Status: Needs review » Needs work
Issue tags: +Needs tests

Good catch, see also #2834316: Node preview shows and defaults to "Default" instead of "Full" view mode for more preview theming/template fun.

Should be easy to add an assertion in an existing node preview test.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

LoMo’s picture

LoMo CreditAttribution: LoMo as a volunteer commented
Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
drupal-node_type_preview_class-2736391-6-tests.patch778 bytes
drupal-node_type_preview_class-2736391-6-complete.patch1.67 KB

I've added an assertion to the PagePreviewTest. It correctly finds the new node type class added on the preview body element.

The patch containing only tests should FAIL (correctly). The "complete" patch should pass and includes the original fix for the issue as well as the new tests. Hope we can get this moved along to RTBC, ASAP. ;-)

LoMo’s picture

LoMo CreditAttribution: LoMo as a volunteer commented
FileSize
drupal-node_type_preview_class-2736391-7-tests.patch779 bytes
drupal-node_type_preview_class-2736391-7-complete.patch1.67 KB

Before the nitpicking police show up and point out that my comment was missing a period at the end... ;-)

(Fix according to Drupal standards. Ignore patches uploaded in #6, though functionally the same.)

The last submitted patch, 6: drupal-node_type_preview_class-2736391-6-tests.patch, failed testing.

The last submitted patch, 7: drupal-node_type_preview_class-2736391-7-tests.patch, failed testing.

LoMo’s picture

LoMo CreditAttribution: LoMo as a volunteer commented

Re #8 and #9, this is what we hoped to see... the tests fail if the patch is not applied. With the fix and tests together (drupal-node_type_preview_class-2736391-7-complete.patch), tests pass. Since the difference between the #6 complete and #7 complete is trivial (missing period added to end inline comment, per Drupal coding standards), we have seen the new test pass twice with the patch.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Anas_maw’s picture

Anas_maw CreditAttribution: Anas_maw as a volunteer commented
Status: Needs review » Reviewed & tested by the community

The patch in 7 worked for me perfectly, this should be set to RTCB
Thank you.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record

Thank you for your work on the issue! This change will cause small change for the node_type variable in the html template, but I think it is reasonable to assume that the preview page and the node page are rendered with same parameters. Let's create a small change record stating that the node_type variable is now set also on the preview pages.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Issue tags: +Novice
leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: Unassigned » leolandotan

I'll work on the change record.

leolandotan’s picture

leolandotan CreditAttribution: leolandotan as a volunteer and at Promet Source commented
Assigned: leolandotan » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs change record

I have created the change record in Add node type to node preview page body class.

Hope everything is in order.

Thanks!

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
CreditAttribution: lauriii at Acquia commented
Status: Needs review » Reviewed & tested by the community

Thanks for creating the change record. Looks good!

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch at Third and Grove commented
Status: Reviewed & tested by the community » Fixed

Committed 2f9be00 and pushed to 8.6.x. Thanks!

  • catch committed 2f9be00 on 8.6.x 
    Issue #2736391 by LoMo, rooby, lauriii, leolando.tan: Node type body...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.