HansSchouten / PHPageBuilder

A drag and drop page builder to manage pages in any PHP project
https://www.phpagebuilder.com
MIT License
730 stars 180 forks source link

styles and scripts are lost #38

Closed dsultanr closed 3 years ago

dsultanr commented 3 years ago

here is my block's content

<style type="text/css">
    #bac-carousel-wrapper {
        position: fixed;
        top: 0;
        left: 0;
        right: 0;
        bottom: 0;
        z-inedx: -1;
    }
    .bac-carousel-item {
        background: center center/cover no-repeat;
        z-index: 0;
        width: 100%;
        height: 100%;
        position: absolute;
        -webkit-transition: 1.3s opacity;
        transition: 1.3s opacity;
        opacity: 0;
    }

    .bac-carousel-item.active {
        opacity: 1;
    }
</style>

<div id="bac-carousel-wrapper">
    <div class="bac-carousel-item active" style="background-image: url('/images/slide-bg1.jpg');"></div>
    <div class="bac-carousel-item" style="background-image: url('/images/slide-bg2.jpg');"></div>
    <div class="bac-carousel-item" style="background-image: url('/images/slide-bg3.jpg');"></div>
</div>

<script type="text/javascript">
    try {
        const bacCarouselItems = document.querySelectorAll(".bac-carousel-item");
        let itemNum = 1;
        setInterval(() =>{
            if(itemNum == bacCarouselItems.length) {
                itemNum = 0;
                bacCarouselItems.forEach(item => item === bacCarouselItems[itemNum] ? bacCarouselItems[itemNum].classList.add("active") : item.classList.remove("active"));
                itemNum = 1;
            } else {
                bacCarouselItems.forEach(item => item === bacCarouselItems[itemNum] ? bacCarouselItems[itemNum].classList.add("active") : item.classList.remove("active"));
                itemNum++;
            }
        }, 10000);
    } catch {}
</script>

Seems my scripts are removed from blocks on save. Is that neccesary? I need custom script for some blocks, how should I add them correctly to block?

Also GrapesJS moved my inline styles to external style but neither new IDs nor classes are not used


<div id="bac-carousel-wrapper" class="IDKHLQN54ECOJFT1">
    <div class="bac-carousel-item active IDKHLQN54PVQJPB2"></div>
    <div class="bac-carousel-item IDKHLQN54TE7U8Q3"></div>
    <div class="bac-carousel-item IDKHLQN54WJ6MQX4"></div>
<style>
    #ieeou{background-image:url('/images/slide-bg1.jpg');}
    #izso2{background-image:url('/images/slide-bg2.jpg');}
    #ijyki{background-image:url('/images/slide-bg3.jpg')
</style>
HansSchouten commented 3 years ago

I see scripts are indeed removed on save. This might be something that GrapesJS is doing in the background, but I am not yet sure. Anyway, you can create a script.js file in your block folder and this should work just fine.

dsultanr commented 3 years ago

I see scripts are indeed removed on save. This might be something that GrapesJS is doing in the background, but I am not yet sure. Anyway, you can create a script.js file in your block folder and this should work just fine.

Yes, script.js inside a block's folder did the trick

Regarding styles - I did patch vendor/hansschouten/phpagebuilder/src/Modules/GrapesJS/resources/views/grapesjs/config.php

'avoidInlineStyle' => true, to 'avoidInlineStyle' => false,

and inline styles began to work. Could you please add this option to phpb_config? Or any other solution?

dsultanr commented 3 years ago

Just noticed - <styles> block also lost

is style.css should work in my block folder as a script.js you mentioned before?

dsultanr commented 3 years ago

I see scripts are indeed removed on save. This might be something that GrapesJS is doing in the background, but I am not yet sure. Anyway, you can create a script.js file in your block folder and this should work just fine.

script.js inside the block' folder succesfully embedded in public state as this code

<script type="text/javascript" class="script2595779758">
document.getElementsByClassName("script2595779758")[0].addEventListener("run-script", function() {
    let inPageBuilder = false;
    let block = document.getElementsByClassName("script2595779758")[0].previousSibling;
    let blockSelector = "." + block.className;
        try {
            const bacCarouselItems = document.querySelectorAll(".bac-carousel-item");
            let itemNum = 1;
            setInterval(() =>{
                if(itemNum == bacCarouselItems.length) {
                    itemNum = 0;
                    bacCarouselItems.forEach(item => item === bacCarouselItems[itemNum] ? bacCarouselItems[itemNum].classList.add("active") : item.classList.remove("active"));
                    itemNum = 1;
                } else {
                    bacCarouselItems.forEach(item => item === bacCarouselItems[itemNum] ? bacCarouselItems[itemNum].classList.add("active") : item.classList.remove("active"));
                    itemNum++;
                }
            }, 10000);
        } catch {}
    });
</script>

but run-script never fired

any thoughts?

dsultanr commented 3 years ago

I did some patching again and it works

https://github.com/HansSchouten/PHPageBuilder/blob/09140815121baa351c354426dcf72b6ca7f92fa4/src/Modules/GrapesJS/Block/BlockRenderer.php#L163-L175

        protected function wrapScriptWithScopeAndContextData($script)
    {
        $scriptId = 'script' . rand(0, 10000000000);
        $html = '<script type="text/javascript" class="' . $scriptId . '">'. PHP_EOL;
        // $html .= 'let inPageBuilder = false;'. PHP_EOL;
        $html .= 'let block'.$scriptId.' = document.getElementsByClassName("' . $scriptId . '")[0].previousSibling;'. PHP_EOL;
        $html .= 'let blockSelector'.$scriptId.' = "." + block'.$scriptId.'.className;'. PHP_EOL;
        $html .= 'var items = document.querySelectorAll(blockSelector'.$scriptId.');';
        $html .= '  for (var i = 0, len = items.length; i < len; i++) {';
        $html .= '    (function(){';
        $html .= '      // START component code'. PHP_EOL;
        $html .= $script;
        $html .= '      // END component code'. PHP_EOL;
        $html .= '    }.bind(items[i]))();';
        $html .= '  }';
        $html .= '</script>';
        return $html;
    }

sample code for block's scripts embedding got from here: https://grapesjs.com/docs/modules/Components-js.html#basic-scripts

HansSchouten commented 3 years ago

The run scripts is fired if you add this to the very bottom of your layout files:

<script type="text/javascript">
    document.querySelectorAll("script").forEach(function(scriptTag) {
        scriptTag.dispatchEvent(new Event('run-script'));
    });
</script>
HansSchouten commented 3 years ago

The problem with the other approach is that you don't have access to any scripts that will be loaded at the end of your page body. By triggering these events, you do have access to them because now your block scripts are runned at the very end. However, I like the binding trick to allow using this. And you can always use a document.ready or something if needed. I will keep this in mind, but for now triggering the run-script event should also work.

dsultanr commented 3 years ago

I had no idea yesterday about why run-script is missing. That's why I made this workaround. Thank you for keep in touch :)

One more edge case - I need the script to be inserted inside concrete div tag... How about this?

dsultanr commented 3 years ago

I need the script to be inserted inside concrete div tag

Done this by custom script.js in a block

    var script = document.createElement("script");
    var done = false;
    script.src = 'https://external.js';
    document.getElementsByClassName("address-map")[0].appendChild(script);    
HansSchouten commented 3 years ago

Ok, you are right. I should add that snippet to the readme or at least the boilerplate. Yes, you can add the script in specific locations that way. So everything is working now?

dsultanr commented 3 years ago

yep. thanks :)

dsultanr commented 3 years ago

Regarding styles - I did patch vendor/hansschouten/phpagebuilder/src/Modules/GrapesJS/resources/views/grapesjs/config.php

'avoidInlineStyle' => true, to 'avoidInlineStyle' => false,

and inline styles began to work. Could you please add this option to phpb_config? Or any other solution?

this issue is left out of the discussion scope. consider please

HansSchouten commented 3 years ago

You are right. However, I was using that setting before but it gave all kinds of issues with saving styles. This setting would save all inline style, but consider what happens if you are using an image slider or other blocks with advanced JS and then you open a page in the pagebuilder and click save. These plugins will add all kinds of style to your html and this would all be saved. And there were more issues as well. Using inline style is a bad practice, please add the appropriate selectors to your html and refer to them in an external stylesheet.

dsultanr commented 3 years ago

moreover I have seen somewhere on GrapesJS's github that this option will be forced to true in future releases

HansSchouten commented 3 years ago

Ok, thanks for the heads up. Once they would build websites as complex as my clients while still offering very easy component script notations, inline styling would definitely break when saving pages. Or scripts should be disabled in the pagebuilder, but that makes editing html inside tab components like here impossible. For my clients saving pages with extensive JS/CSS works fine without needing any inline styles. The problem of using inline styles is that JS packages would add styling into the DOM that should not be stored.