Testing out the templating with breakdownTimeline.php #2489
Conversation
www/breakdownTimeline.php
Outdated
| echo view('pages.breakdownTimeline', [ | ||
| //'test_results_view' => true, // Not Sure if we need this | ||
| 'body_class' => 'result', | ||
| // 'results_header' => $results_header, // Null Value, Not sure if we need this |
There was a problem hiding this comment.
In the consolelog.php case $results_header is the contents of include_once 'header.inc' but with output buffering. Because I need the content but it should come after the overall header which comes with the default template.
Otherwise the test results header will come before the top menu in my experience.
There was a problem hiding this comment.
I see what you mean, it was to fix the layout of the header. Currently without using $result_header my header looks like the image above.
Would you still recommend I pass in $result_header like you did in consolelog.php even though it is ok?
like this ?
ob_start();
define('NOBANNER', true); // otherwise Twitch banner shows 2x
$tab = 'Test Result';
$subtab = 'Processing';
include_once 'header.inc';
$results_header = ob_get_contents();
ob_end_clean();
Hey thanks a lot for all the great feedback! |
sammeboy635
force-pushed
the
breakdownTimeline
branch
from
bbdcc1f
to
282ada3
Compare
sammeboy635
force-pushed
the
breakdownTimeline
branch
from
282ada3
to
4aef63a
Compare
|
It's ready for review :). WebPageTest.-.Google.Chrome.2022-10-27.11-41-25.mp4 |
| // $page_keywords = array('Timeline Breakdown','WebPageTest','Website Speed Test','Page Speed'); | ||
| // $page_description = "Chrome main-thread processing breakdown$testLabel"; |
There was a problem hiding this comment.
You can move these as globals to the .php file (outside the template)
They should just work. If not we'll fix in a follow up diff
| if (isset($groups) && is_array($groups) && count($groups)) { | ||
| foreach ($groups as $type => $time) { |
There was a problem hiding this comment.
there are loops and ifs in Blade, could you try them?
https://laravel.com/docs/9.x/blade#if-statements
https://laravel.com/docs/9.x/blade#loops
the idea is to minimize <?php as much as we can
There was a problem hiding this comment.
And echo is really what we don't need when we have templates :)
| echo "groups.setValue($index, 1, $time);\n"; | ||
| $color = $groupColors[$type]; | ||
| echo "groupColors.push('$color');\n"; | ||
| $index++; |
There was a problem hiding this comment.
check out the magical $loop var:
https://laravel.com/docs/9.x/blade#the-loop-variable
It gives you $loop->index :)
| var groupsIdle = new google.visualization.DataTable(); | ||
| groupsIdle.addColumn('string', 'Category'); | ||
| groupsIdle.addColumn('number', 'Time (ms)'); | ||
| groupsIdle.addRows(<?php echo count($groups); ?>); |
There was a problem hiding this comment.
I think these count calls should work in hte template too, like
groupsIdle.addRows({{ count($groups) }});
|
I was going to push another PR for this but I was thinking to clean up the php we could process everything on the JS side. This way we are not passing 200+ lines of Let me know if you would rather me go the larvel templating method instead. |
|
yeah, I like the idea! But also I wanted to replace all these google API-powered graphs and pies with something self-hosted, throughout the site. Something like https://www.chartjs.org/ |
@stoyan I was hoping to get some help on this if you wouldn't mind but I'm a little unsure how to PR this cleanly.
I did the bare minimum to just get breakdownTimeline.php to work with the templater and it seems to work just fine. Do you have thoughts on it?
Preview with templater
