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

Don't output preview containers what also affects live #5

Closed
maapteh opened this issue Aug 30, 2018 · 5 comments
Closed

Don't output preview containers what also affects live #5

maapteh opened this issue Aug 30, 2018 · 5 comments

Comments

@maapteh
Copy link

maapteh commented Aug 30, 2018

Hi, if i look at for example 'CmsContainer' i see a lot of code which also affects live output but also things which can be optimized. I don't really like the extra div's its creating for example (for live output, less is more), and also 'preview' is passed inside this component while lot of it could be done outside.

https://github.com/bloomreach/experience-react-sdk/blob/master/src/cms-components/core/container.js#L6

when i create something like this (without looping just an example of what i mean):


import React from 'react';
import CmsContainerItem from './container-item';
import { addBeginComment, addEndComment } from '../../utils/add-html-comment';
import { PreviewContext } from '../../context';

// TODO: component can become stateless
export default class CmsContainer extends React.Component {

	if (!this.props.configuration) {
	  return null;
	}

	render() {
		return (
			<PreviewContext.Consumer>
			{ preview =>
				<React.Fragment>
					{ renderContainerWrapper(this.props.configuration, preview) }
				</React.Fragment>
			}
			</PreviewContext.Consumer>
		);
	};
}

const renderContainerWrapper = (configuration: any, preview: boolean = false) => {

	// TODO: move to generic preview component
	const Preview = (props: any) => {

		if (!preview) return props.children;

		const addMetaData = (htmlElm: any) => {
			addBeginComment(htmlElm, 'beforebegin', configuration, preview);
			addEndComment(htmlElm, 'afterend', configuration, preview);
		}

		return (
			<div
				className="hst-container"
				data-cms-id={configuration.id}
				ref={addMetaData}
			>
				{props.children}
			</div>
		)
	};

	return (
		<Preview>
			<CmsContainerItem configuration={configuration} />
		</Preview>
	);

}

I have no containers as output which is only needed for PREVIEW. Also I don't need to go trough functions which are only needed when in preview mode (faster). I don't need to have reference.

I like the sdk, don't get me wrong but i would just to point out some things I dislike about it but will be happy to work trough them if my company decides to take this route.

@TimoWestland
Copy link
Contributor

I agree that only rendering the hst-container and hst-container-item divs in preview mode would be a very nice improvement to the SDK!

@robbertkauffman
Copy link
Contributor

Hi,

Thanks for reporting and sorry for the delayed response, somehow I didn't receive any notifications on new issues.

I fully agree that this can indeed be optimized. I'll make some adjustments. Are you willing to test out the adjustments when I publish a new RC/beta tag?

Best,
Robbert

@robbertkauffman
Copy link
Contributor

robbertkauffman commented Sep 14, 2018

Hi @maapteh and @TimoWestland ,

I have made some modifications to the container and container-item classes so they no longer insert CMS-required markup outside of preview. See 8e9d79b.

You can test the changes in the latest beta tag (0.3.0-rc1), see: https://www.npmjs.com/package/bloomreach-experience-react-sdk/v/0.3.0-rc1

Let me know if this fixes it!

Best,
Robbert

@maapteh
Copy link
Author

maapteh commented Sep 17, 2018

Hi Robbert,

I ended up writing my own HOC because in my situation the PageModel is not the same as Hippo anymore, maybe it can "help" your 'if else' stuff a little more nicer. Thanks for you changes and response anyway.

import * as React from 'react';
import { addBeginComment, addEndComment } from '@common/helper/edit.helper';

export const containerPreview = (WrappedComponent: any) => {

    class HOC extends React.Component<any> {

        addMetaData = (htmlElm: any) => {
            addBeginComment(htmlElm, this.props.configuration, 'afterbegin');
            addEndComment(htmlElm, this.props.configuration, 'beforeend');
        }

        render() {
            return (
                <div
                    className="cms-preview-container"
                    data-cms-id={this.props.configuration.id}
                    ref={this.addMetaData}
                >
                    <WrappedComponent
                        {...this.props}
                    />
                </div>
            );
        }

    }

    return HOC;
};

consumer:
const CmsContainerWrapper = preview ? containerPreview(CmsContainerItem) : CmsContainerItem;

@robbertkauffman
Copy link
Contributor

Thanks for sharing!

I'm closing this issue assuming this fixes it. Anyone, feel free to reopen if the solution is not working or causing issues.

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

No branches or pull requests

3 participants