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

Multiple dimension loops? #195

Open
cxj opened this issue May 9, 2018 · 38 comments
Open

Multiple dimension loops? #195

cxj opened this issue May 9, 2018 · 38 comments

Comments

@cxj
Copy link
Contributor

cxj commented May 9, 2018

Is there anyway in TSS to loop through more than one dimension of a data structure? For example, if my data contained:

array(
    1 => array(
        1 => 'ThingOne AttributeOne',
        2 => 'ThingOne AttributeTwo',
    ),
    2 => array(
        1 => 'ThingTwo AttributeOne',
        2 => 'ThingTwo AttirbuteTwo',
    ),
    // Etc.
);

I'd like a way to loop through and display all the strings above. Is this possible? If not, I can work out a way to flatten my data structure before passing it to Transphporm.

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

I would guess your outer loop would be repeat: data(); and then you'd establish your inner loop with repeat: iteration();, accessing each string with content: iteration();.

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

I've been trying variations on that theme, but with no luck. For example, this:

ol li {
    repeat: data(output.benefit_data.eb);
}
tr {
    repeat: iteration();
}
td span.benefit {
    content: key();
}
td span.value {
    content: iteration();
}

Results in this fatal error:

DOMDocument::createTextNode() expects parameter 1 to be string, object given
line 75 in file ../vendor/level-2/transphporm/src/Property/Content.php

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

Can you isolate which TSS line is causing that error (I would guess it's one of the last two) by commenting one out at a time?

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

It's the second iteration() that causes it to blow up. The key() does not cause a fatal error, but it also does not output the correct array index value, either.

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

Interesting. What is key() outputting?

I would like to see what your output looks like if you change that line in Content.php from this:

$new->appendChild($document->createTextNode($node));

to this:

$new->appendChild($document->createTextNode(get_class($node)));

because apparently it's not a DomElement, DomComment, or a DomText.

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

key() outputs a numeric value 1 less than the numeric index of the outer loop. In my real data (versus simplified example at top), the inner array indices are actually strings, so it's pretty obvious I'm not getting them.

Give me a few minutes to edit Content.php and I'll make your suggested change and post the results.

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

So key() just seems to return an offset or iteration counter rather than the actual key. 🤔

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

Yes, that's what it seems like.

Results of Content.php change (line number changed by one because original is commented out):

get_class() expects parameter 1 to be object, string given
line 76 in file ../vendor/level-2/transphporm/src/Property/Content.php

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

Oh, that makes sense. My fault.
Change that line back to what it was, and then after this line:

if ($node instanceof \DomText) $node = $node->nodeValue;

add this:

else if (is_object($node)) $node = get_class($node);

I think that should do what I was intending.

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

Ok, did that I think, if I managed to correctly follow your instructions. ;-)

Output for the iteration() is now the string stdClass for each value in the array.

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

Ok, maybe that will be helpful in tracking down the issue. I'm not all that familiar with the code base but I do know that @TomBZombie likes to use plain objects sometimes, so I'm sure Transphporm is internally creating those somewhere for some reason. I'll dig around the code a bit until he gets here 😉

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

Actually, I just realized my real data contains a mix of arrays and objects, which I was just treating as an array of arrays, since Transphporm mostly does the same. Maybe that was a faulty assumption.

That is, it appears the real data looks like this:

array(
    1 => stdClass(
        'foo' => 'ThingOne AttributeOne',
        'bar' => 'ThingOne AttributeTwo',
    ),
    2 => stdClass(
        'foo' => 'ThingTwo AttributeOne',
        'bar' => 'ThingTwo AttirbuteTwo',
    ),
    // Etc.
);

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

I can see how you would assume that. What happens if you make it just arrays?

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

I'm trying to figure out how to do that now. I'm displaying data that I receive from an API as JSON, so I'll need to write some PHP code to walk the data structure and convert objects to arrays, I think.

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

Ultimately, I don't think you should have to do that, but at least for testing purposes it might be helpful.

My best guess right now is that the issue lies in \Transphporm\Property\Content::getNode() because it only tests is_array() to determine whether recursion is necessary.

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

If I do the most simple minded thing of changing your else if code above to read this way:

else if (is_object($node)) {
    // $node = get_class($node);
   $node = (array)$node;
}

Then I get this error:

DOMDocument::createTextNode() expects parameter 1 to be string, array given
line 90 in file ../vendor/level-2/transphporm/src/Property/Content.php

Line 90 in my hacked up code is the same place errors occurred earlier:

            $new->appendChild($document->createTextNode($node));

I guess really no surprise there. I will attempt to convert the objects to arrays in my data before calling Transphporm and see how that goes.

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

I wonder...
What if you change that line back to using get_class(), and then change the test in getNode() from:

if (is_array($n)) {

to this:

if (is_array($n) || (is_object($n) && $n instanceof \stdClass)) {

If that does what I think it does, you shouldn't see stdClass in the output at all, but rather your data.

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

Making that change gets me the value parts of the stdClass attributes, but not the keys. Also, they're all concatenated together in one string, rather than separate rows. Hmm. But at least I'm getting data out and it's not aborting.

@cxj
Copy link
Contributor Author

cxj commented May 9, 2018

Ah, it seems it is iterating through my entire $data object rather than the subcomponent I specified in the my TSS rule:

li {
    repeat: data(output.benefit_data.eb);
}

@garrettw
Copy link
Contributor

garrettw commented May 9, 2018

I think this is where, having completed the discovery phase, I leave it in Tom's capable hands. 😉

@TRPB
Copy link
Member

TRPB commented May 13, 2018

Can you give me a complete example?

One of the issues here is that transphporm sorts rules by specificity so the following rules:

ol li {
    repeat: data(output.benefit_data.eb);
}
tr {
    repeat: iteration();
}
td span.benefit {
    content: key();
}
td span.value {
    content: iteration();
}

Will get executed in this order:

  1. tr
  2. ol li
  3. td span.benefit
  4. td span.value

With the following script:

<?php
require 'vendor/autoload.php';

$data = array(
    1 => [
        	'foo' => 'ThingOne AttributeOne',
        	'bar' => 'ThingOne AttributeTwo'
    ],
    2 => [
        'foo' => 'ThingTwo AttributeOne',
        'bar' => 'ThingTwo AttirbuteTwo'
    ]
);

$tss = '
ol li {
	repeat: data();
}
tr {
    repeat: iteration();
}
td span.benefit {
    content: key();
}
td span.value {
    content: iteration();
}
';

$xml = '
<ol>
	<li>

		<table>
			<tr>
			    <td><span class="benefit"></span></td>
			    <td><span class="value"></span></td>
			</tr>
		</table>
	</li>
</ol>
';

$tpl = new Transphporm\Builder($xml, $tss);
echo $tpl->output($data)->body;
echo "\n";

This is the output:

<ol>
        <li>

                <table>
                        <tr>
                            <td><span class="benefit">1</span></td>
                            <td><span class="value">ThingOne AttributeOneThingOne AttributeTwo</span></td>
                        </tr>
<tr>
                            <td><span class="benefit">1</span></td>
                            <td><span class="value">ThingOne AttributeOneThingOne AttributeTwo</span></td>
                        </tr>
                </table>
        </li>
<li>

                <table>
                        <tr>
                            <td><span class="benefit">2</span></td>
                            <td><span class="value">ThingTwo AttributeOneThingTwo AttirbuteTwo</span></td>
                        </tr>
<tr>
                            <td><span class="benefit">2</span></td>
                            <td><span class="value">ThingTwo AttributeOneThingTwo AttirbuteTwo</span></td>
                        </tr>
                </table>
        </li>
</ol>

The <tr>s are created first, then the whole <li> with the containing table is then looped over. What you probably want this this:

li {
	repeat: data();
}
tr {
    repeat: iteration();
}
td span.benefit {
    content: key();
}
td span.value {
    content: iteration();
}

Which now produces the correct output:

<ol>
        <li>

                <table>
                        <tr>
                            <td><span class="benefit">foo</span></td>
                            <td><span class="value">ThingOne AttributeOne</span></td>
                        </tr>
<tr>
                            <td><span class="benefit">bar</span></td>
                            <td><span class="value">ThingOne AttributeTwo</span></td>
                        </tr>
                </table>
        </li>
<li>

                <table>
                        <tr>
                            <td><span class="benefit">foo</span></td>
                            <td><span class="value">ThingTwo AttributeOne</span></td>
                        </tr>
<tr>
                            <td><span class="benefit">bar</span></td>
                            <td><span class="value">ThingTwo AttirbuteTwo</span></td>
                        </tr>
                </table>
        </li>
</ol>

In 99% of cases we need to run outer blocks before inner blocks, consider this:

main nav li {
	repeat: data(nav);
}

main  { 
	content: template('foo.xml');
}

The second rule must be executed first.

We can't really prioritise by content type (e..g content: template() or repeat()) because I might want to include a template for each iteration or iterate over an included template.

It might be worth to add a z-index style property for allowing specifying rule execution order in the TSS file.

@garrettw
Copy link
Contributor

Specifying rule execution order could help, but...

I believe Chris approached this from the same logical standpoint that I did; namely, that selector specificity would not be the determining factor for loop nesting, but rather where each element falls in the DOM tree. In other words, it seems logical that if a selector of any specificity targets a given element with a repeat attribute, then any descendant with a matching selector that uses iteration() would then receive an array element from that loop -- unless there is a repeat: iteration() at some interstital level, which would cause iteration()s below that to pull from deeper levels of the array, and so on.

I anticipate this being a point of confusion for consumers of this library until DOM level alone is made the determining factor for nesting.

@TRPB
Copy link
Member

TRPB commented May 13, 2018

Because of content: template() we don't really know DOM Level until the DOM is rendered which causes a chicken and egg problem.

Consider the following:

main { 
 content: template('page.xml');
}

.sidebar li { 
  repeat: data(navigation);
}

If the .sidebar element is included as part of page.xml we can't sort rules by DOM Level, because we don't know the DOM level of .sidebar li until after the rule for main has already run.

It would be possible to avoid this by working the other way around: Rather than looping through the rules and finding any element that matches the rules, we could iterate over every element in the document and then find any rules that apply to it. However, this would come at rather large expense to performance and make caching considerably less efficient as you'd still need to loop through every element.

@TRPB
Copy link
Member

TRPB commented May 13, 2018

Looking at this:

ol li {
    repeat: data(output.benefit_data.eb);
}
tr {
    repeat: iteration();
}
td span.benefit {
    content: key();
}
td span.value {
    content: iteration();
}

It would be possible to look for things like data and iteration in the rules then use that as part of the sorting algorithm so that anything with data is executed before anything with iteration but this may also introduce problems in cases where you have nested loops or complex pages.

@cxj
Copy link
Contributor Author

cxj commented May 13, 2018

Tom and Garrett: thanks for continuing the discussion. I'm really busy this weekend with non-programming projects, so I hope to get back to working on this on Monday. I'll post here when I have something of potential value.

@garrettw
Copy link
Contributor

@TomBZombie I'd like to throw one more wrench into this issue, but I thought better of it so I'll be making it a separate thing.

@cxj
Copy link
Contributor Author

cxj commented May 14, 2018

I tried rearranging my HTML to use the specificity order that Tom's example used, and that worked for my current data structure. Despite my knowledge of CSS, knowing the specificity order that Transphporm will use to evaluate rules is difficult.

@TRPB
Copy link
Member

TRPB commented May 15, 2018

knowing the specificity order that Transphporm will use to evaluate rules is difficult.

It will always execute by specificity so selectors with more parts will be executed first.

div will always be executed before div span. Although this isn't always what's needed (e.g. for loops) it is sometimes required.

The reason for this is that, like CSS, you can have multiple rules that affect the same element.

Consider:

div span {content: "B"; }
span { content: "A"; }

the span element's content is set twice. Following CSS's standards, the more specific rule should override the less specific one and you'd expect the element's content to be set to "B".

@cxj
Copy link
Contributor Author

cxj commented May 15, 2018

I guess I missed the reality that my table rows were more specific than the list items, which enclosed the table. I was thinking that

td span.benefit

was really only more specific than

tr { ... }

completely missing that even the naked tr was more specific than the

ol li { ... }

rule.

I guess all we need is a bit of documentation in the Wiki on this, and the ticket can be closed.

@garrettw
Copy link
Contributor

What do you mean? How could tr be more specific than ol li?

@cxj
Copy link
Contributor Author

cxj commented May 15, 2018

Because the <tr> element is embedded within the <li> element. That is, tr is really ol li table tr, right?

Or it could be that I'm completely confused in trying to understand how and why Transphporm processes the rules in the order that it does. :-p

I think it's probably the latter. I am confused.

@garrettw
Copy link
Contributor

Following CSS specificity rules, tr is less specific because it applies to any tr anywhere on the page -- and therefore it is processed first. If you need the tr to be part of the loop, then its selector needs to be more specific than the selector for the loop wrapper (in this case, ol li). So rewriting tr to be ol li tr would make it more specific and thus make the loop work in the way you intended.

@cxj
Copy link
Contributor Author

cxj commented May 15, 2018

Thanks. It makes perfect sense when I'm working with CSS, but for some reason I get muddled when working with repeat() and content: iteration().

@cxj
Copy link
Contributor Author

cxj commented May 15, 2018

Maybe I should write the Wiki documentation for this. If I get it right there, (checked by you and Tom?), then I'll never have to worry again. :-)

@TRPB
Copy link
Member

TRPB commented May 15, 2018

I'm going to improve the way it works and allow arbitrary ordering and not using specificity for everything.

I think this should work:

  • Loop through all the rules (without sorting them)
  • Find any elements that match the rule and do $element->addRule($rule) and then at $element to a list of elements which have rules applied to them
  • Loop through all elements and calculate depth in the DOM tree then sort them
  • Loop through all elements Execute all rules applied to each element sorting by specificity
  • Repeat the process if any rules called template() to apply rules to elements which have been added to the document since the first run

@cxj
Copy link
Contributor Author

cxj commented May 15, 2018

I'm not sure you need to make any changes. With some documentation, specificity works fine. Which consumes fewer resources?

@TRPB
Copy link
Member

TRPB commented May 15, 2018

How it is now will be faster, but with caching enabled won't make much difference.

@garrettw
Copy link
Contributor

That new logic seems sound. I like it.

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