[MGNLFE-35] Control layout of greenbars and components within an area Created: 21/Apr/20  Updated: 23/Jun/20  Resolved: 27/May/20

Status: Closed
Project: Magnolia Frontend Helpers
Component/s: None
Affects Version/s: 1.0.0
Fix Version/s: 1.0.4

Type: Improvement Priority: Major
Reporter: Bartosz Staryga Assignee: Canh Nguyen
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: 0d
Time Spent: 1.5d
Original Estimate: Not Specified

Attachments: PNG File Screenshot 2020-05-04 at 11.20.46.png     PNG File area-bar-and-component.png    
Template:
Acceptance criteria:
Empty
Task DoD:
[ ]* Doc/release notes changes? Comment present?
[ ]* Downstream builds green?
[ ]* Solution information and context easily available?
[ ]* Tests
[ ]* FixVersion filled and not yet released
[ ]  Architecture Decision Record (ADR)
Documentation update required:
Yes
Date of First Response:
Epic Link: SPA Editor
Sprint: HL & LD 3
Story Points: 3

 Description   

As a developer I want to be able to control the layout of components in an area, and the green bars of the area and the components.

Options (see comments):

  • additional component to act as area itself
  • adding an attribute to editable area to state which element is the component container / div

Original proposal:

Area component in front end editors for SPA should be wrapped in DIV with class prop.

Area component e.g. in react-editor is not wrapped in any div.
That results in green bars being added on the same level as components.

This is ok when element wrapping area does not implement any display property e.g. flex. If property is added green bars and components are all affected/

Solution would be to make area be a div and be able to use class prop.

E.g https://git.magnolia-cms.com/projects/MODULES/repos/frontend-helpers/browse/packages/react-editor/src/component/EditableArea/EditableArea.js#51

To sth like:

<div className={this.props.className}>
{componentNames.map((name) => this.renderComponentWithComment(name))}
</div>


 Comments   
Comment by Christopher Zimmermann [ 21/Apr/20 ]

Agree

Comment by Christopher Zimmermann [ 04/May/20 ]

Change should not affect existing projects, but new projects should be able to opt-in.

Comment by Christopher Zimmermann [ 04/May/20 ]

bstaryga can you please provide an example case of the issue? Would help team come up with a solution.

Comment by Bartosz Staryga [ 04/May/20 ]

In the existing minimal demo in here [https://git.magnolia-cms.com/projects/DEMOS/repos/minimal-headless-spa-demos/browse/spa/react-minimal/src/pages/Basic.js#12
]change 

<main>

to

<main style={{ display: 'flex' }}>

And you will end up with this:

If area components would be wrapped in a div green bars would be rendered correctly and I could still have flex display of all components inside the area.

 

Comment by Mikaël Geljić [ 04/May/20 ]

Ok I got a (freemarker) example which we can relate maybe:

Parent element with specific display could always affect the area bar in freemarker world too.
Adding the wrapper in the project is typically a no-brainer. This ensures having the exact same visual result when editing vs. viewing too. Otherwise you might end up with editor/preview-specific selectors, could have unwanted side-effects too.

Could still consider a page-editor directive/attribute instructing whether the area bar should be injected "around" the area, vs. inside or with addition of a wrapping div. But then letting template developer choose (e.g. as an explicit attribute of EditableArea / same on freemarker directives).

Comment by Bartosz Staryga [ 04/May/20 ]

mgeljic `Adding the wrapper in the project is typically a no-brainer` is not solution here.
If I wanna add display: flex in my area I can not right now. Cause if I add wrapper in my JSX the green bars will be still rendered inside my wrapper and affected by display style.

It gets in a way if dev wants components to be in a columns. e.g. all sliders. Sure they are workarounds for it, but let's try to go for sth that is nice to use not sth that will need workarounds for simplest examples.

Comment by Bartosz Staryga [ 04/May/20 ]

On freemarker we use the area template script to sort this out. And we use it quite often maybe simple prop wrapwith or anything to specify a html elements to wrap components with and its class. It'd be a big help. I am.doing the e-commerce demo just now and had to change how I implement components because of that exact issue.

Comment by Mikaël Geljić [ 04/May/20 ]

Copy that, yeah I was thinking towards the area script too; ok, so goal would be to unlock freedom of scripting the area itself, let define what div should act as the component slot.

Comment by Bartosz Staryga [ 04/May/20 ]

That would be full blown solution

Comment by Bartosz Staryga [ 04/May/20 ]

mgeljic I'm assuming that with that full blown solution you'd export also `Component` which devs would be able to use in their areas renderers. It would be quite the treat as it would sort the other issue I came across just now - campaigns.
Campaign returns the component node but wrapped in campaign component making it not easy (needs from dev to duplicating what lib component is already doing) to render correctly.

Comment by Christopher Zimmermann [ 04/May/20 ]

mgeljic I agree that we would not want a differnet rendering for editing vs previewing, were you suggesting that bstaryga?

bstaryga RE "On freemarker we use the area template script to sort this out.", Do you find that a good pattern? I am worried that it adds complexity and that it could usually be handled with your 'wrapwith' suggestion, or even just the 'areas always add divs, and if a className is passed then it gets put on that div' approach?

To ask in another way, Re: "And we use it quite often", do you use that control in different ways - or basically always just to add that wrapping div with a class?

Comment by Bartosz Staryga [ 05/May/20 ]

czimmermann most often it it for exactly adding certain classes to areas, passing component index, so that component knows which one is it in area (https://jira.magnolia-cms.com/browse/SUGGEST-223) and wrapping components in some element.
For starter it would be perfectly fine with simple wrapping + class prop. Doing the demos now, I am missing this. Would I use full blow render area? Probably not just yet. Down the road it could come in handy but it's more 'nice to have'.

Generated at Mon Feb 12 05:43:36 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.