Problem/Motivation

From #2959493: Allow Layout Builder live previews to be toggled to allow easier drag-and-drop item 5
When live preview mode is disable in the Layout Builder UI, the placeholder labels are added via Javascript. When live preview mode is re-enabled, the labels are removed from the DOM by javascript. It would be simpler for maintainers and other developers if these labels were always present and conditionally visible based on a parent class.

Proposed resolution

Figure out a way to add the labels to Layout Builder blocks inside the UI that have a structure consistent enough to facilitate visibility based on only CSS rules. Currently, the content of .layout-builder-block .content is not consistent enough to safely target with CSS, and sometimes includes child elements that are not wrapped in tags.

This will be made much easier if the placeholder labels are siblings of .layout-builder-block .content instead of children.

The current placeholder logic is in JS and BlockComponentRenderArray::onBuildRender.

Remaining tasks

Determine the best approach to add a consistently structured content-preview-disabled placeholder label in Layout Builder blocks. Make these conditionally visible based on the the presence of .layout-builder--content-preview-disabled wrapping the Layout Builder UI Implement, then remove the JS and other code responsible for adding/removing the labels previously.

User interface changes

UI will remain the same

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

CommentFileSizeAuthor
#26 3043215-nr-bot_gohvjp3m.txt98 bytesneeds-review-queue-bot
#20 3043215-20.patch11.21 KBklu
#17 3043215-nr-bot.txt143 bytesneeds-review-queue-bot
#6 3043215-6.patch34.84 KBbnjmnm
#6 interdiff_3-6.txt32.01 KBbnjmnm
#3 3043215--3.patch17.01 KBbnjmnm

Issue fork drupal-3043215

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

bnjmnm created an issue. See original summary.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Title: Create permanent placeholder element for content-preview-disabled placeholder labels. » Create non-js placeholder element for content-preview-disabled placeholder labels.
Issue summary: View changes
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Active » Needs review
StatusFileSize
new 3043215--3.patch17.01 KB

This significantly reduces the JS part of the content preview toggle functionality. This required adding wrappers to a few elements, which seems like a fine tradeoff for reducing JS.

For the reviewer:

  • Was not sure if the .layout-builder-block__content-preview-show visibility class was not proper BEM. Figured it was fastest to post as-is and find out what needs changing in the review
  • There was a bit of JS refactoring required beyond removing the parts that were no longer needed. I replaced the $layoutBuilder variable with direct calls to $('#layout-builder') to address an issue caught by tests. After dragging blocks to a new position, the $layoutBuilder would become stale. Referencing a fresh $('#layout-builder') took care of it.

Status: Needs review » Needs work

The last submitted patch, 3: 3043215--3.patch, failed testing. View results

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Assigned: Unassigned » bnjmnm
bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
StatusFileSize
new interdiff_3-6.txt32.01 KB
new 3043215-6.patch34.84 KB

The version in #3 attempted to control visibility based only on the presence of a wrapper class. It became apparent this approach would not work after testing with the Search form block. The CSS for the Search form block includes setting several elements to display: inline. This overrides the css-based visibility rules. While it was tempting to just open a followup for the search block, this would still be a problem for any block that has elements with display: explicitly set. To hide content reliably, using .hide() and .show() in javascript is required. The Search form block was added to tests since it exposed problems that needed to be addressed.

This patch does successfully make the preview labels a part of the render array instead of dynamically created/removed via javascript. This approach required additional test coverage to account for structural differences in default layouts vs overrides and the need to explicitly address block titles.

bnjmnm’s picture

bnjmnm
he/him
Primary language English
Location Ann Arbor, MI
commented
Status: Needs work » Needs review
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Assigned: bnjmnm » Unassigned
Issue tags: +JavaScript

Unassigning and tagging to get some reviews

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.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Category: Feature request » Task
Issue tags: -JavaScript +JavaScript

Not as much a feature request as a task

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.

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.

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.

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

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3043215-nr-bot.txt143 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

cboyden’s picture

cboyden commented

The placeholder labels still cause an issue when trying to style the LB interface. Is the approach in this ticket still being considered, or are there other proposed solutions?

klu’s picture

klu commented
StatusFileSize
new 3043215-20.patch11.21 KB

Thanks for your work on this, @bnjmnm! The changes in your patch would be really helpful for us. I’ve taken a stab at re-rolling for 10.3.x based on patch 3 and the interdiff. It appears to work for me and is a significant improvement for doing our styling. I’m attaching the patch that I’m using, but I did not try to update the tests (my apologies; I’m newly coming back to Drupal development).

danielveza’s picture

danielveza
he/him
Location Brisbane, AU
commented

Hey @klu, could you please create a merge request from the patch that you've created against the 11.x branch?

There is some documentation here about how to create merge requests for Drupal issues.

Thanks!

cboyden’s picture

cboyden commented

I've created an issue fork from 11.x and ported the changes from patch #20. Still needs updated tests.

cboyden opened merge request !10290

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

cboyden’s picture

cboyden commented
Status: Needs work » Needs review

Merged changes from 11.x branch to MR.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3043215-nr-bot_gohvjp3m.txt98 bytes

The Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.