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

fix: prevent unintended CSS inheritance on DropZone content #885

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

DamianKocjan
Copy link
Contributor

This pull request addresses a potential styling issue introduced by the recent change where DropZones are now wrapped in a <div>. Previously, users could directly style the container element of a component that included a DropZone (e.g., <section className="sectionBlock"><DropZone /></section>). The new wrapping <div> can cause unexpected inheritance of styles from parent elements, breaking existing layouts.

Problem:

With the new <div> wrapper, CSS rules targeting the parent container (e.g., section.sectionBlock) now also apply to the wrapping <div> and subsequently, to the content rendered within the DropZone. This can lead to unintended styling changes.

Example:

Consider the following component configuration:

const config = {
  components: {
    ComponentA: {
      render() {
        return <section className="sectionBlock"><DropZone zone="component-a-zone" /></section>;
      }
    },
    ComponentB: {
      render() {
        return <div>ComponentB</div>;
      }
    }
  }
};

Old Puck Version (Correct Behavior):

<section class="sectionBlock">
  <div>ComponentB</div>
</section>

New Puck Version (Incorrect Behavior - Unintended Styling):

<section class="sectionBlock">
  <div>  <!-- New wrapping div -->
    <div>ComponentB</div>
  </div>
</section>

In the new version, styles applied to section.sectionBlock might unintentionally affect the content within the DropZone due to the intermediate <div>.

Solution:

This pull request introduces the as prop to the DropZone component. This prop allows users to specify the HTML element to be used as the DropZone's container. By setting as="section", users can maintain the original structure and prevent unintended style inheritance.

Example Usage:

<DropZone zone="component-a-zone" as="section" />

This change allows users to have more control over the styling of their DropZones and ensures backward compatibility with existing component configurations. It avoids breaking existing layouts that rely on specific CSS rules targeting parent containers.

Copy link

vercel bot commented Feb 7, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
puck-docs ✅ Ready (Inspect) Visit Preview Feb 12, 2025 10:55am

Copy link

vercel bot commented Feb 7, 2025

@DamianKocjan is attempting to deploy a commit to the Measured Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

Thanks for this PR @DamianKocjan!

Some context: The previous version of Puck rendered inconsistently between the and environments. The Puck environment included a div, but the Render environment didn't. This resulted in inconsistencies between each environment.

Having said that, this as API is quite a nice addition and I can see why it might help, but I am concerned that it approach opens the door to other element types that might have their own user-agent styles that conflict with the DropZone styles and behaviour (li, table, input, etc). Although this API does mean that you don't need to update your class selectors, it doesn't provide a seamless upgrade path as the user still needs to specify the as.

Interested to hear your thoughts.

In future, we're expecting the slots API to be introduced in favour of DropZones (see #255). If we're going with as, we'll likely need to solve that for slots too.

@DamianKocjan
Copy link
Contributor Author

I think it's easier to add extra styles for user-agent styles than to tweak css that's not in your control. We could add note with warning that style side effect might appear due to user-agent style.

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