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

Nested Components #41

Open
holloway opened this issue May 19, 2019 · 2 comments
Open

Nested Components #41

holloway opened this issue May 19, 2019 · 2 comments

Comments

@holloway
Copy link

Hi I see that #39 had a fix for children / nested components, has that been released yet?

I'm just testing this code and I've found that it produces odd HTML where certain sections are repeated.

With this usage,

<:FlexContainer width="fixed">
	<:FlexRow>
		<:FlexColumn xs="4" sm="4" md="4" lg="4">
			column1
		</:FlexColumn>
		<:FlexColumn xs="4" sm="4" md="4" lg="4">
			column2
		</:FlexColumn>
		<:FlexColumn xs="4" sm="4" md="4" lg="4">
			column3
		</:FlexColumn>
	</:FlexRow>
</:FlexContainer>

And these templates:

FlexContainer.ss

<div class="<% if $width == "fixed" %>g-flex-container<% else_if $width == "fluid" %>g-flex-container-fluid<% end_if %>"> 
  $children
</div>

FlexRow.ss

<div class="g-flex-row<% if $alignXs == "start" %>start-xs<% else_if $alignXs == "center" %>center-xs<% else_if $alignXs == "end" %>end-xs<% end_if %> <% if $alignSm == "start" %>start-sm<% else_if $alignSm == "center" %>center-sm<% else_if $alignSm == "end" %>end-sm<% end_if %> <% if $alignMd == "start" %>start-md<% else_if $alignMd == "center" %>center-md<% else_if $alignMd == "end" %>end-md<% end_if %> <% if $alignLg == "start" %>start-lg<% else_if $alignLg == "center" %>center-lg<% else_if $alignLg == "end" %>end-lg<% end_if %> <% if $isReversed %> g-flex-reverse<% end_if %>"> 
  $children
</div>

FlexColumn.ss

<div class="g-flex-col <% if $xs == "1" %>g-flex-col-xs-1<% else_if $xs == "2" %>g-flex-col-xs-2<% else_if $xs == "3" %>g-flex-col-xs-3<% else_if $xs == "4" %>g-flex-col-xs-4<% else_if $xs == "5" %>g-flex-col-xs-5<% else_if $xs == "6" %>g-flex-col-xs-6<% else_if $xs == "7" %>g-flex-col-xs-7<% else_if $xs == "8" %>g-flex-col-xs-8<% else_if $xs == "9" %>g-flex-col-xs-9<% else_if $xs == "10" %>g-flex-col-xs-10<% else_if $xs == "11" %>g-flex-col-xs-11<% else_if $xs == "12" %>g-flex-col-xs-12<% else_if $xs == "auto" %>g-flex-col-xs<% end_if %> <% if $sm == "1" %>g-flex-col-sm-1<% else_if $sm == "2" %>g-flex-col-sm-2<% else_if $sm == "3" %>g-flex-col-sm-3<% else_if $sm == "4" %>g-flex-col-sm-4<% else_if $sm == "5" %>g-flex-col-sm-5<% else_if $sm == "6" %>g-flex-col-sm-6<% else_if $sm == "7" %>g-flex-col-sm-7<% else_if $sm == "8" %>g-flex-col-sm-8<% else_if $sm == "9" %>g-flex-col-sm-9<% else_if $sm == "10" %>g-flex-col-sm-10<% else_if $sm == "11" %>g-flex-col-sm-11<% else_if $sm == "12" %>g-flex-col-sm-12<% else_if $sm == "auto" %>g-flex-col-sm<% end_if %> <% if $md == "1" %>g-flex-col-md-1<% else_if $md == "2" %>g-flex-col-md-2<% else_if $md == "3" %>g-flex-col-md-3<% else_if $md == "4" %>g-flex-col-md-4<% else_if $md == "5" %>g-flex-col-md-5<% else_if $md == "6" %>g-flex-col-md-6<% else_if $md == "7" %>g-flex-col-md-7<% else_if $md == "8" %>g-flex-col-md-8<% else_if $md == "9" %>g-flex-col-md-9<% else_if $md == "10" %>g-flex-col-md-10<% else_if $md == "11" %>g-flex-col-md-11<% else_if $md == "12" %>g-flex-col-md-12<% else_if $md == "auto" %>g-flex-col-md<% end_if %> <% if $lg == "1" %>g-flex-col-lg-1<% else_if $lg == "2" %>g-flex-col-lg-2<% else_if $lg == "3" %>g-flex-col-lg-3<% else_if $lg == "4" %>g-flex-col-lg-4<% else_if $lg == "5" %>g-flex-col-lg-5<% else_if $lg == "6" %>g-flex-col-lg-6<% else_if $lg == "7" %>g-flex-col-lg-7<% else_if $lg == "8" %>g-flex-col-lg-8<% else_if $lg == "9" %>g-flex-col-lg-9<% else_if $lg == "10" %>g-flex-col-lg-10<% else_if $lg == "11" %>g-flex-col-lg-11<% else_if $lg == "12" %>g-flex-col-lg-12<% else_if $lg == "auto" %>g-flex-col-md<% end_if %> <% if $xsOffset == "0" %>g-flex-col-xs-offset-0<% else_if $xsOffset == "1" %>g-flex-col-xs-offset-1<% else_if $xsOffset == "2" %>g-flex-col-xs-offset-2<% else_if $xsOffset == "3" %>g-flex-col-xs-offset-3<% else_if $xsOffset == "4" %>g-flex-col-xs-offset-4<% else_if $xsOffset == "5" %>g-flex-col-xs-offset-5<% else_if $xsOffset == "6" %>g-flex-col-xs-offset-6<% else_if $xsOffset == "7" %>g-flex-col-xs-offset-7<% else_if $xsOffset == "8" %>g-flex-col-xs-offset-8<% else_if $xsOffset == "9" %>g-flex-col-xs-offset-9<% else_if $xsOffset == "10" %>g-flex-col-xs-offset-10<% else_if $xsOffset == "11" %>g-flex-col-xs-offset-11<% else_if $xsOffset == "12" %>g-flex-col-xs-offset-12<% end_if %> <% if $smOffset == "0" %>g-flex-col-sm-offset-0<% else_if $smOffset == "1" %>g-flex-col-sm-offset-1<% else_if $smOffset == "2" %>g-flex-col-sm-offset-2<% else_if $smOffset == "3" %>g-flex-col-sm-offset-3<% else_if $smOffset == "4" %>g-flex-col-sm-offset-4<% else_if $smOffset == "5" %>g-flex-col-sm-offset-5<% else_if $smOffset == "6" %>g-flex-col-sm-offset-6<% else_if $smOffset == "7" %>g-flex-col-sm-offset-7<% else_if $smOffset == "8" %>g-flex-col-sm-offset-8<% else_if $smOffset == "9" %>g-flex-col-sm-offset-9<% else_if $smOffset == "10" %>g-flex-col-sm-offset-10<% else_if $smOffset == "11" %>g-flex-col-sm-offset-11<% else_if $smOffset == "12" %>g-flex-col-sm-offset-12<% end_if %> <% if $mdOffset == "0" %>g-flex-col-md-offset-0<% else_if $mdOffset == "1" %>g-flex-col-md-offset-1<% else_if $mdOffset == "2" %>g-flex-col-md-offset-2<% else_if $mdOffset == "3" %>g-flex-col-md-offset-3<% else_if $mdOffset == "4" %>g-flex-col-md-offset-4<% else_if $mdOffset == "5" %>g-flex-col-md-offset-5<% else_if $mdOffset == "6" %>g-flex-col-md-offset-6<% else_if $mdOffset == "7" %>g-flex-col-md-offset-7<% else_if $mdOffset == "8" %>g-flex-col-md-offset-8<% else_if $mdOffset == "9" %>g-flex-col-md-offset-9<% else_if $mdOffset == "10" %>g-flex-col-md-offset-10<% else_if $mdOffset == "11" %>g-flex-col-md-offset-11<% else_if $mdOffset == "12" %>g-flex-col-md-offset-12<% end_if %> <% if $lgOffset == "0" %>g-flex-col-lg-offset-0<% else_if $lgOffset == "1" %>g-flex-col-lg-offset-1<% else_if $lgOffset == "2" %>g-flex-col-lg-offset-2<% else_if $lgOffset == "3" %>g-flex-col-lg-offset-3<% else_if $lgOffset == "4" %>g-flex-col-lg-offset-4<% else_if $lgOffset == "5" %>g-flex-col-lg-offset-5<% else_if $lgOffset == "6" %>g-flex-col-lg-offset-6<% else_if $lgOffset == "7" %>g-flex-col-lg-offset-7<% else_if $lgOffset == "8" %>g-flex-col-lg-offset-8<% else_if $lgOffset == "9" %>g-flex-col-lg-offset-9<% else_if $lgOffset == "10" %>g-flex-col-lg-offset-10<% else_if $lgOffset == "11" %>g-flex-col-lg-offset-11<% else_if $lgOffset == "12" %>g-flex-col-lg-offset-12<% end_if %> <% if $isReversed %> g-flex-reverse<% end_if %>"> 
  $children
</div>

I get HTML with incorrect textNodes and nesting of

<div class="g-flex-container"> 
	column3
	<div class="g-flex-col g-flex-col-xs-4 g-flex-col-sm-4 g-flex-col-md-4 g-flex-col-lg-4     "> 
		column3
	</div>
	<div class="g-flex-row"> 
		column3
		<div class="g-flex-col g-flex-col-xs-4 g-flex-col-sm-4 g-flex-col-md-4 g-flex-col-lg-4     "> 
			column3
		</div>
	</div>
</div>

Or does this render correctly for you with these templates?

Also, will fix #39 be published for SilverStripe 3 and 4?

Thanks

@holloway
Copy link
Author

I've been looking into this and using the ?showtemplate=1 param to see the PHP generated.

It seems to be improved somewhat by changing ComponentService.php's handleChildHTML to be,

 private static function handleChildHTML($data, $properties, $parser)
    {
        if (isset($data['Children']['php'])) {
            // handle children property collision
            if (isset($properties['children'])) {
                throw new SSTemplateParseException('Cannot use "children" as a property name and have inner HTML.', $parser);
            }

            // construct php
            $value = $data['Children']['php'];
            $php = "\$_props['children'] = '';\n";  
          
            $php = str_replace("\$_props = array();\n", "", $php);
            $php = str_replace("unset(\$_props);\n", "", $php);

            $valAppend = "\$val .=";
            if(strpos($value, $valAppend) !== false) {
                // replace only the last
                $php .= substr_replace($value, "\$_props['children'] .=", strrpos($php, $valAppend), strlen($valAppend));
            }

            $php .= "\$_props['children'] = SilverStripe\ORM\FieldType\DBField::create_field('HTMLText', \$_props['children']);\n";
            
            return $php;
        }
        return '';
    }

This seems to fix the textNodes in my test cases.

However the row and container are still printed afterwards, without the correct nesting. And now it's not printing the FlexContainer class 😕

The new input now looks like this,

<:FlexContainer width="fixed">
  <:FlexRow>
    <:FlexColumn xs="4" sm="4" md="4" lg="4">
      column1
    </:FlexColumn>
    <:FlexColumn xs="8" sm="8" md="8" lg="8">
      column22
    </:FlexColumn>
  </:FlexRow>
</:FlexContainer>

and the new output looks like,

<div class="g-flex-col g-flex-col-xs-4 g-flex-col-sm-4 g-flex-col-md-4 g-flex-col-lg-4"> 
      column1
</div>
<div class="g-flex-col g-flex-col-xs-8 g-flex-col-sm-8 g-flex-col-md-8 g-flex-col-lg-8"> 
     column22
</div>
<div class="g-flex-row"> 
</div>
<div class=""> 
</div>

@nglasl
Copy link
Contributor

nglasl commented Oct 1, 2019

@JoshuaCarter any ideas here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants