Introduced in #2095275: Space above add content form with no overlay, safe to remove because supporting logic had a bug that made it a no-op anyway.

Original report:

  if ($suggestions = theme_get_suggestions($path_args, 'page', '-')) {
    foreach ($suggestions as $suggestion) {
      if ($suggestion === 'page-node-edit' || strpos($suggestion, 'page-node-add') !== FALSE) {
        $variables['attributes']['class'][] = 'node-form-layout';
      }
    }
  }

should be: 'page--node-edit' and 'page--node-add'

CommentFileSizeAuthor
#35 Screenshot 2022-04-27 at 6.07.55 PM.png551.95 KBajaypratapsingh
#32 2844714.32.patch1.38 KBsakthivel m
#23 2844714-23-alt-rm-obsolete.patch1.4 KBtacituseu
#22 8-5-x.JPG21.88 KBVidushi Mehta
#22 8-4-x.JPG25.27 KBVidushi Mehta
#16 2844714-16-alt-rm-obsolete.patch1.4 KBtacituseu
#15 2844714-15-alt-rm-obsolete.patch2.23 KBtacituseu
#11 2844714-11-alt-rm-obsolete.patch2.42 KBtacituseu
#9 2844714-alt-remove-obsolete-code.patch1.59 KBtacituseu
#8 after-patch.png125.97 KBbander2
#8 before-patch.png125.92 KBbander2
#2 2844714-2.patch702 bytestacituseu

Issue fork seven-2844714

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

tacituseu created an issue. See original summary.

tacituseu’s picture

tacituseu commented
Status: Active » Needs review
StatusFileSize
new 2844714-2.patch702 bytes

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.

bander2’s picture

bander2 commented
Status: Needs review » Reviewed & tested by the community

I can confirm that without the patch the "node-form-layout" class is not added to the body of node edit pages as seems to be the purpose of the hook. The patch fixes the issue.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs review

So I guess this means we should be asking the question is core/themes/seven/css/layout/node-add.css actually needed and what is it doing since the class it depends on is never added? Maybe some screenshots before and after this fix are in order? It's possible this patch actually breaks something :)

tacituseu’s picture

tacituseu commented
Related issues: +#2095275: Space above add content form with no overlay

Shuffled quite a bit over time but #2095275: Space above add content form with no overlay seems to be the origin.

tacituseu’s picture

tacituseu commented
Related issues: +#2234331: Change the body classes to follow Drupal 8 CSS standards, +#2330661: Node form layout bug when toolbar collapsed

Also some mention here #2330661-10: Node form layout bug when toolbar collapsed.

bander2’s picture

bander2 commented
StatusFileSize
new before-patch.png125.92 KB
new after-patch.png125.97 KB

The node-form-layout class is found in only one other place in the source. in core/themes/seven/css/layout/node-add.css:

@media
  screen and (min-width: 780px),
  (orientation: landscape) and (min-device-height: 780px) {
  .node-form-layout .messages {
    margin-top: 1em;
    margin-bottom: 1em;
  }
}

And we do find that before applying the patch, the .messages div on the node edit page has a margin of 9px 0 10px 8px. After the patch, 1em 0 1em 8px.

tacituseu’s picture

tacituseu commented
StatusFileSize
new 2844714-alt-remove-obsolete-code.patch1.59 KB

No harm in removing it then...

Status: Needs review » Needs work

The last submitted patch, 9: 2844714-alt-remove-obsolete-code.patch, failed testing.

tacituseu’s picture

tacituseu commented
Status: Needs work » Needs review
StatusFileSize
new 2844714-11-alt-rm-obsolete.patch2.42 KB

... thoroughly ;)

bander2’s picture

bander2 commented
Status: Needs review » Reviewed & tested by the community

Relevant markup is identical before and after the patch. Looks good.

star-szr’s picture

star-szr
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs title update, +Needs issue summary update

Thanks everyone, I think the issue summary/title need an update based on what I'm seeing in the patch and recent comments.

tacituseu’s picture

tacituseu commented
Title: Wrong suggestion strings checked in seven_preprocess_html() » Remove obsolete overlay styles and supporting logic in seven_preprocess_html()
Issue summary: View changes
Issue tags: -Needs title update, -Needs issue summary update

Leaving at NW as in the meantime #2068063: Change "Save and keep un-/published" buttons to a "Published" checkbox and an included "Save" button made some changes to core/themes/seven/css/layout/node-add.css, also #2863431: Change "Save and keep un-/published" buttons for media module is about to modify it again.

tacituseu’s picture

tacituseu commented
Status: Needs work » Needs review
StatusFileSize
new 2844714-15-alt-rm-obsolete.patch2.23 KB

Leaving core/themes/seven/css/layout/node-add.css in place, just removed the obsolete style.

tacituseu’s picture

tacituseu commented
StatusFileSize
new 2844714-16-alt-rm-obsolete.patch1.4 KB

... and leaving lib too.

The last submitted patch, 15: 2844714-15-alt-rm-obsolete.patch, failed testing. View results

manuel garcia’s picture

manuel garcia commented

Re #14 - #2863431: Change "Save and keep un-/published" buttons for media module is not going to modify node-add.css.

manuel garcia’s picture

manuel garcia commented

Also, if we want to remove this file, could we perhaps move it to css/components/node.css ?

tacituseu’s picture

tacituseu commented

The issue that introduced the file: #2321505: Split Seven's style.css into SMACSS categories.

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.

Vidushi Mehta’s picture

Vidushi Mehta commented
StatusFileSize
new 8-4-x.JPG25.27 KB
new 8-5-x.JPG21.88 KB

I applied the patch after pulling the latest code on 8.4.x and 8.5.x but on both branches i got an error.
PFA the screenshots after applying the patch.

tacituseu’s picture

tacituseu commented
StatusFileSize
new 2844714-23-alt-rm-obsolete.patch1.4 KB

Re-rolled.

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.

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

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.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.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.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.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

sakthivel m’s picture

sakthivel m commented
Status: Needs review » Needs work
Issue tags: +Needs reroll

#23 Patch Failed

sakthivel m’s picture

sakthivel m commented
Status: Needs work » Needs review
StatusFileSize
new 2844714.32.patch1.38 KB

#32 Please review the patch

yogeshmpawar’s picture

yogeshmpawar
He/Him/His
Primary language English
Location Pune(MH), INDIA 🇮🇳
commented
Issue tags: -Needs reroll

Removing Needs reroll tag as it is no longer needed.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

ajaypratapsingh’s picture

ajaypratapsingh commented
StatusFileSize
new Screenshot 2022-04-27 at 6.07.55 PM.png551.95 KB

#32 patch is working fine on Drupal 9.3.x and can be moved to RTBC

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Project: Drupal core » Seven
Version: 9.5.x-dev » 1.0.0-alpha1
Component: Seven theme » Code

The Seven theme has been removed from Drupal 10 core. I confirmed that this issue only affects Seven and no other themes included with Drupal core, so I am moving this to the contributed Seven project.

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Version: 1.0.0-alpha1 » 1.0.x-dev
Status: Needs review » Needs work
Issue tags: +Needs merge request
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: Unassigned » avpaderno
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: +Needs reroll

avpaderno opened merge request !62

avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs work » Needs review
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Issue tags: -Needs merge request, -Needs reroll

  • avpaderno committed be99cbb3 on 1.0.x 
    Issue #2844714: Remove obsolete overlay styles and supporting logic in...

  • avpaderno committed 7af39128 on 2.0.x 
    Issue #2844714: Remove obsolete overlay styles and supporting logic in...
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Status: Needs review » Fixed
avpaderno’s picture

avpaderno
he/him
Primary language Italian
Location Brescia, 🇮🇹 🇪🇺
commented
Assigned: avpaderno » Unassigned

Status: Fixed » Closed (fixed)

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