[MGNLFE-97] Greenbars rendering with "display: flex" (Flexbox) Created: 01/Mar/21 Updated: 06/Jan/22 Resolved: 12/Jul/21 |
|
| Status: | Closed |
| Project: | Magnolia Frontend Helpers |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | 1.1.0 |
| Type: | Improvement | Priority: | Neutral |
| Reporter: | Bartosz Staryga | Assignee: | Canh Nguyen |
| Resolution: | Done | Votes: | 0 |
| Labels: | VN-Analysis, VN-Implementation, VN-Testing | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 7d 4.25h | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||
| Issue Links: |
|
||||||||
| Template: |
|
||||||||
| Acceptance criteria: |
Empty
|
||||||||
| Task DoD: |
[X]*
Doc/release notes changes? Comment present?
[X]*
Downstream builds green?
[X]*
Solution information and context easily available?
[X]*
Tests
[X]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||
| Release notes required: |
Yes
|
||||||||
| Documentation update required: |
Yes
|
||||||||
| Date of First Response: | |||||||||
| Epic Link: | SPA Editor Backlog | ||||||||
| Sprint: | HL & LD 32 | ||||||||
| Story Points: | 5 | ||||||||
| Description |
|
"display: flex" css has become popular among front end developers. Current rendering of Greenbars is making it hard/impossible to use Flexbox. Eg. Greenbars would be rendered next to components.
In ftl we can go around it by adding custom templateScript for an area and wrap the component loop in div with flex. Create a simple page/app that illustrates the problem. |
| Comments |
| Comment by Bartosz Staryga [ 21/Jun/21 ] |
|
lam.nguyen canh.nguyen if we do not want to touch the Magnolia bits then adding templateScript should do the trick. So custom area, yes, but consistent syntax with what was in ftl I did a small test and it works with it like a charm. What I have tried is this: Extended EditableArea.js
render() {
const { content, className, elementType, children, templateScript } = this.props;
const componentNames = content['@nodes'];
const element = React.createElement(elementType || 'div');
return (
<element.type
ref={(node) => (this.node = node)}
key={content['@id']}
className={ComponentHelper.classnames(className)}
>
{children}
{templateScript
? templateScript(content)
: componentNames.map((name) => <EditableComponent key={content[name]['@id']} content={content[name]} />)}
</element.type>
);
}
Using EditableArea import React from 'react'; import { EditableArea, EditableComponent } from '@magnolia/react-editor'; function templateScript(content) { return ( <div style={{ display: 'flex' }}> {content['@nodes'].map((name) => ( <EditableComponent key={content[name]['@id']} content={content[name]} /> ))} </div> ); } function Home(props) { const { title, main, metadata } = props; return ( <div> <h1>{title}</h1> {main && ( <EditableArea content={main} parentTemplateId={metadata['mgnl:template']} templateScript={templateScript} /> )} </div> ); } export default Home; And with this we have nice workaround for display flex as in ftl. |
| Comment by Lam Nguyen Bao [ 21/Jun/21 ] |
|
Hi bstaryga, It's great to hear from you. Actually, we tried to have solutions with minimal impact to page structure. So, I do believe that using custom template is a correct approach. Indeed, it must be supported for others as well. |
| Comment by Canh Nguyen [ 22/Jun/21 ] |
|
Hi bstaryga, I'm glad that it worked for you. I'm aware of the templateScript of ftl but I would not recommend to support it in react-editor or the rest. The templateScript in template definition is processed on the server side and it is difficult to use it on SPA. Because react needs a component to render, not a string. It will be easier for both developer or Magnolia if we let developer decides what he wants to display the data by extending EditableEditor. It would be nice if we have a blog post for this problem on our website, so that our customers can learn from it. And I think we can close this ticket. WDYT? |
| Comment by Bartosz Staryga [ 24/Jun/21 ] |
|
Hmmm... frankly I disagree with you Let's remember that great bunch of devs using headless already know magnolia and also the idea of template script is natural for them. czimmermann 5 cents from you? |
| Comment by Christopher Zimmermann [ 25/Jun/21 ] |
|
I'd like to have as much frontender input on this as possible. stas your input here would be much appreciated! |
| Comment by Senol Tas [ 28/Jun/21 ] |
|
Not sure I fully understand the issue. Extending the EdititableArea (I guess that what lam.nguyen is suggesting) would be from my perspective counter productive. Also we need to document the approach and give the possibility, the actual extension point should be in the core of the product.
Having a templateScript property looks odd for me, I think there are some react patterns established on how to deal with this situations. I think if we go this route it might look like this:
import React from 'react'; import { EditableArea, EditableComponent } from '@magnolia/react-editor'; function Home(props) { const { title, main, metadata } = props; return ( <div> <h1>{title}</h1> {main && ( <EditableArea content={main} parentTemplateId={metadata['mgnl:template']}> {({content})=>( <div style={{display: 'flex'}}> {content['@nodes'].map((name) => ( <EditableComponent key={content[name]['@id']} content={content[name]} /> ))} </div> )} </EditableArea> )} </div> ); } export default Home; But honestly that's for me personally already to verbose. Perhaps something like that would be better: import React from 'react'; import { EditableArea, EditableAreaContent, EditableComponent } from '@magnolia/react-editor'; function Home(props) { const { title, main, metadata } = props; return ( <div> <h1>{title}</h1> {main && ( <EditableArea content={main} parentTemplateId={metadata['mgnl:template']}> {({content})=>( <div style={{display: 'flex'}}> <EditableAreaContent content={content}/> </div> )} </EditableArea> )} </div> ); } export default Home; |
| Comment by Canh Nguyen [ 30/Jun/21 ] |
|
I got the idea from Bartosz, I would introduce a new customView property than a templateScript. Because in this case it's not like the templateScript we used in ftl. Please check my commit here https://git.magnolia-cms.com/projects/MODULES/repos/frontend-helpers/commits/ee5a0de02514f01232d29b225f4f553bfdc0a7ea#packages/react-editor/src/component/EditableArea/EditableArea.js This is the custom view:
Here is the change in EditableArea
I've tried to remove the wrapping element and let the custom view deciding what would be displayed but it caused another issue. The editor will find elements in the annotation, if there is only one element, it will append the green bar inside that element.
|
| Comment by Lam Nguyen Bao [ 30/Jun/21 ] |
|
IMO, having a particular view for specific problem is not very nice. Later, views would be introduced for other cases. How's about having div as stas's example with a specific container style class id, then user can do whatever they want based on css. |
| Comment by Canh Nguyen [ 30/Jun/21 ] |
|
Hi lam.nguyen I'm not sure if stas's solution is working. From my understanding, inside EditableArea tag is another react component. That component requires a content parameter. In the EditableArea we can get that component by accessing through props.children . There are another problem that we should treat all children generally, and we don't know if each child needs any parameters to pass in. You can see this line https://git.magnolia-cms.com/projects/MODULES/repos/frontend-helpers/browse/packages/react-editor/src/component/EditableArea/EditableArea.js#72 that we support to render EditableArea's children. |
| Comment by Senol Tas [ 30/Jun/21 ] |
|
canh.nguyen I think it is valid to discuss the approach, there are many pros and cons. The big pro for me is the developer experience, think we need to add that into account. There are certain things you expect from a library as a consumer and if we can make that more convenient I'd rather rate it higher. The question I'd have would be why should i add children to a EditableArea in the first palce. Do we have use-cases for that? From functionality perspective it sound odd to me, i can just add my custom components above EditableArea. If children is a map, that's the default case when nesting components nobody is expecting any magic property passing at all, but if you put in a function it's clear. A hacky working solution to demonstrate: // .... render() { const { content, className, elementType, children } = this.props; const componentNames = content['@nodes']; const element = React.createElement(elementType || 'div'); return ( <element.type ref={node => this.node = node} key={content['@id']} className={ComponentHelper.classnames(className)}> {typeof children === 'function' ? this.props.children({content}) : children } { typeof children !== 'function' ? componentNames.map((name) => <EditableComponent key={content[name]['@id']} content={content[name]} />) : null} </element.type> ); } //....
Do not have any preference to be honest. Both works, the children as function will just speed up development I believe.
czimmermann Do we have some partners who already are familiar with the spa editor to do a little survey?
|
| Comment by Christopher Zimmermann [ 30/Jun/21 ] |
|
Great input everyone. Thank you! It's challenging! bstaryga you wrote:
So far we've been discussing adding templateScript kind of thing to the editor libraries... |
| Comment by Christopher Zimmermann [ 30/Jun/21 ] |
|
canh.nguyen Is your customView the same as the templateScript approach, but just with a different name? |
| Comment by Christopher Zimmermann [ 30/Jun/21 ] |
|
I'm not totally opposed to templateScript/customView approach, it seems useful. But it does feel a little clunky and confusing to me when I look at code samples. I have a feeling that wrapping with a div (or other element) with a custom class is the main use case by far... and maybe the only use case. Or does someone see another usecase for the area templateScript? If that is the case, what about adding a new property to the area like `wrapWithClass` or `wrapperClass`? Or two properties: `wrapperElement` and `wrapperClass`? |
| Comment by Canh Nguyen [ 01/Jul/21 ] |
|
TemplateScript is a function, and customView is a component. Actually, a component is also a function but following the framework standard. Developer can freely design their components to pass to EditableArea. Furthermore, I'm not sure if we can do the same for VueJS and Angular with Senol Tas' approach, but passing a component seem to be supported in all frameworks. |
| Comment by Canh Nguyen [ 01/Jul/21 ] |
Sorry for my ignorance, I've just check again and I know that Angular has ContentChildren and Vue has $slots that we may support Senol Tas' approach in all frameworks. I'm trying to implement this approach with VueJS. |
| Comment by Canh Nguyen [ 01/Jul/21 ] |
|
Please check Senol Tas' approach on this commit https://git.magnolia-cms.com/projects/MODULES/repos/frontend-helpers/commits/cec5d5332fe9bf45fb9805efc8662a37115fc06d In order to make the function working, we must call React#createElement or we just create a component like FlexArea component in my example.
With this implementation, we still support the NoComponentArea that we just get the content from the Area dialog to display on the page.
|
| Comment by Christopher Zimmermann [ 01/Jul/21 ] |
|
It's too bad that the frontend developer has to put things in their component code that is specific to the editing experience. Up till now it's just our EditableArea component mostly. Would it make sense to instead of doing this wrapperDiv handling in the ffrontend devs components javascript code, that rather the area definition can specify the customView/template script - and then our editor libraries auto instantiate the component for areas just as it does for components? (Basically the same as the freemarker approach?) |
| Comment by Bartosz Staryga [ 01/Jul/21 ] |
|
czimmermann other uses includes every time you want to do sth extra with components. Wheater it is wrapping the whole area in div, or wrapping each component with an extra element. Another one is making an area with no components but rendering things based on what is in the area dialog. Both are heavily used in e.g. Swiss Re or Corp website and were deeply missed by me when doing headless demo sites I was pushing initially towards the templateScript naming and approach cause a great number of developers who try Magnolia in headless know already Magnolia and are familiar with the concept. Anyway having a render prop passed, or making use of `children` is equally nice. Bottom line we should have an easy solution that is built into the libs. A solution that allows custom rendering of areas. |
| Comment by Canh Nguyen [ 01/Jul/21 ] |
|
WDYT that we add extra prop like this?
[
div: {
cssClass: []
},
ul: {
cssClass: []
}
]
Then we will create extra wrapping elements for the content. The ul tag here is the div child, if we add more elements, the latter is the child of the previous one. |
| Comment by Christopher Zimmermann [ 01/Jul/21 ] |
|
Thanks bstaryga, ok I am onboard for the utitlity of having a render prop or children, rather than my suggestion about `wrapperDiv` etc. For render props vs children, I cannot decide, I think Dominic should make the call based on all of the input and his experiments. If we went with a "render prop" (https://reactjs.org/docs/render-props.html) (I guess that was what Bartosz original proposal was I would prefer either "render" as the name, as that seems like an established pattern, or "customView". I can see that "templateScript" has good consistancy with freemarker, but it really sounds odd to me in the frontend framework project. |
| Comment by Canh Nguyen [ 01/Jul/21 ] |
|
Someone on Stackoverflow recommends that we should use ChildComponent instead of passing a component as a parameter. So I will go with child component approach so that we will implement it to all libraries. |
| Comment by Canh Nguyen [ 05/Jul/21 ] |
|
I've just added a commit that I implemented for Angular, please check it here. In Angular, we can use ng-content to get html code that wrapped inside of EditableArea but we cannot access the data from EditableArea. There're two possible solutions which are dynamic component (the same as we implemented for the angular-editor) and ngTemplate and ngTemplateOutlet. IMO the dynamic component is more complex than the second solution that I implemented. The ComponentChildren I mentioned above cannot be implemented in this case (maybe I haven't found the correct document). The ngTemplate and ngTemplateOutlet solution is that we create a template and name it, then pass it to EditableArea. This is similar to templateScript on FTL. bstaryga stas have you experienced this problem with Angular before? I'm trying to implement the same approach to all frameworks but it seems very hard to do so. |