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

Minor improvements #17

Open
wants to merge 34 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
b3a5875
Improved CRUD basic setup
jadb Feb 20, 2016
90a1ba8
Updated default layout
jadb Feb 20, 2016
b4ba3af
Dropped ie8 support
jadb Feb 20, 2016
7426709
Added some HTML5 Boilerplate stuff
jadb Feb 20, 2016
6e0404a
Forced ie8 to render as edge
jadb Feb 20, 2016
547f347
Added alert for unsupported ie versions
jadb Feb 20, 2016
1edb4cc
Loaded CrudView plugin
jadb Feb 20, 2016
e093c0e
Updated docblock
jadb Feb 20, 2016
226ca7c
Added `webroot/assets` to composer installer
jadb Feb 20, 2016
9be5f7e
Added missing import
jadb Feb 20, 2016
3676145
Ignored `webroot/assets`
jadb Feb 20, 2016
eba5c59
Added the asset compress helper
jadb Feb 20, 2016
13b6154
Switched to bowerphp & bootstrap-sass
jadb Feb 20, 2016
b50b628
Re-order packages
jadb Feb 20, 2016
f41851f
Added modernizr
jadb Feb 20, 2016
c70634c
Improved assets organization
jadb Feb 20, 2016
0b9a05c
Revert assets re-organization
jadb Feb 20, 2016
0f405f6
Simplified bootstrap/bootswatch & sass
jadb Feb 21, 2016
3fad43c
Fixed missing/outdated links & code highlighting
jadb Feb 21, 2016
86b94bf
Named the homepage route
jadb Feb 21, 2016
fb29d6b
Extracted base layout for re-use
jadb Feb 21, 2016
9ba5638
Added `ErrorController`
jadb Feb 21, 2016
cd5c648
Started adding some basic elements
jadb Feb 21, 2016
3e0f67a
Improved menu handling in production
jadb Feb 21, 2016
ae5cdc0
Improved error layout and 400 types
jadb Feb 21, 2016
037e6b6
Disabled CrudView built-in assets
jadb Feb 21, 2016
c5b011b
Dropped Modernizr
jadb Feb 22, 2016
ae9ffe4
Added `bootstrap-wysihtml5`
jadb Feb 22, 2016
7a6adde
Added `ViewVarsListener`
jadb Feb 22, 2016
630ba5c
Use the application `CrudView`
jadb Feb 22, 2016
b3e9347
Dropped concatenation, better readability
jadb Feb 22, 2016
bb21623
Use `container-fluid` in views
jadb Feb 22, 2016
eea41e9
Fixed broken home route
jadb Feb 22, 2016
a0a743a
Updated homepage HTML markup
jadb Feb 22, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 75 additions & 1 deletion src/Controller/AppController.php
Original file line number Diff line number Diff line change
@@ -1,10 +1,84 @@
<?php

namespace App\Controller;

use Cake\Controller\Controller;
use Crud\Controller\ControllerTrait;

class AppController extends Controller
{
use ControllerTrait;

/**
* Tells if the CRUD listeners should be enabled.
*
* @var bool
*/
public $isCrudEnabled = true;

/**
* Tells if CRUD view listener and view should be used.
*
* @var bool
*/
public $isCrudViewEnabled = true;

/**
* Tells if the CRUD API listeners should be enabled.
*
* @var bool
*/
public $isCrudApiEnabled = false;

/**
* {@inheritdoc}
*/
public function initialize()
{
parent::initialize();

$this->loadComponent('Flash');
$this->loadComponent('RequestHandler');

$this->_setupCrud();
}

protected function _setupCrud()
{
if (!$this->isCrudEnabled) {
return;
}

$this->loadComponent('Crud.Crud', [
'listeners' => [
'Crud.Redirect',
'Crud.RelatedModels',
]
]);

$this->_setupCrudView();
$this->_setupCrudApi();
}

protected function _setupCrudView()
{
if (!$this->isCrudViewEnabled) {
return;
}

$this->Crud->addListener('CrudView.View');
$this->viewBuilder()
->className('CrudView.Crud')
->layout('default');
}

protected function _setupCrudApi()
{
if (!$this->isCrudApiEnabled) {
return;
}

$this->Crud->addListener('Crud.Api');
$this->Crud->addListener('Crud.ApiPagination');
$this->Crud->addListener('Crud.ApiQueryLog');
}
}
59 changes: 0 additions & 59 deletions src/Controller/CrudController.php

This file was deleted.

14 changes: 13 additions & 1 deletion src/Controller/PagesController.php
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
<?php

/**
* CakePHP(tm) : Rapid Development Framework (http://cakephp.org)
* Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
*
* Licensed under The MIT License
* For full copyright and license information, please see the LICENSE.txt
* Redistributions of files must retain the above copyright notice.
*
* @copyright Copyright (c) Cake Software Foundation, Inc. (http://cakefoundation.org)
* @link http://cakephp.org CakePHP(tm) Project
* @since 0.2.9
* @license http://www.opensource.org/licenses/mit-license.php MIT License
*/
namespace App\Controller;

use Cake\Core\Configure;
Expand Down
38 changes: 7 additions & 31 deletions src/Template/Layout/default.ctp
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
<!DOCTYPE html>

<?php
echo $this->fetch(
'html',
sprintf('<html lang="%s" class="no-js">', read('App.language'))
);
?>
<html lang="<?= \Locale::getPrimaryLanguage(\Cake\I18n\I18n::locale()) ?>">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting Content Language Header and this is very imporant fir the browser nowadays, css hyphens uses those. Great stuff.


<head>

Expand All @@ -15,38 +9,20 @@ echo $this->fetch(
<?= $this->fetch('title', read('App.title', env('HTTP_HOST'))) ?>
</title>

<meta name="viewport" content="width=device-width, initial-scale=1">
<meta name="author" content="<?= read('App.author') ?>">
<?= $this->Html->meta('icon') ?>
<?= $this->fetch('meta') ?>


<?= $this->AssetCompress->css('platform') ?>
<?= $this->fetch('css'); ?>
<!-- Le HTML5 shim, for IE6-8 support of HTML5 elements -->
<!--[if lt IE 9]>
<script src="//html5shim.googlecode.com/svn/trunk/html5.js"></script>
<script src="//oss.maxcdn.com/libs/respond.js/1.3.0/respond.min.js"></script>
<![endif]-->
<?= $this->fetch('css') ?>
<?= $this->fetch('headjs') ?>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not work with CRUD since CakePHP2 / CRUDv3 (custom build crud view). So bear with me if this comment has more the form of an RFC. I don't know what <?= $this->AssetCompress->css('platform') ?> does, if it includes below ignore:

I suggest adding the following:

  1. core-application-top.css
  2. core-application-top.js
  3. some-plugin-application-top.css
  4. some-plugin-application-top.js
  5. core/pluginname-controllername-actionname-top.css
  6. core/pluginname-controllername-acitonname-top.js
  7. core-application-bottom.css
  8. core-application-bottom.js
  9. some-plugin-application-bottom.css
  10. some-plugin-application-bottom.js
  11. core/pluginname-controllername-actionname-bottom.css
  12. core/pluginname-controllername-actionname-bottom.js

E.g.

  1. for core css and js application top, and bottom are being loaded, same for each plugin (if called controller is part of that plugin).
  2. each action (prefixed by controller and plugin (or nothing for core?) gets its css and js loaded
    No statements if files don't exist and/or plugins and/or controllers and/or actions are not part of the request

Usually you should by doing so go well with one application-top.css, one application-bottom.css, one application-bottom.js, one action-bottom.css and one action-bottom.js. 5 requests but those allow dereferencing deferring and decoupling. The application-* files will be in get cache anyway and thus only action-bottom.css and action-bottom.js will need to be loaded (2 requests after the first full page load). One could opt-in for eager loading by using prefixes (body class="pluginname-controllername-actionname" is the easiest) and just application-top.css and application-bottom.js.

What I suggest here should work well with eager-loading, but also lazy-loading and decoupling, with preprocessor pipelines and tries to strike limiting download size of css/js files as well as the amount of request (request-overhead). It works with conventions out of the box and if you don't use certain files (because they either don't exist or are not relevant to the request) those files won't even appear.

What happens with elements/cells and css/js is not part of this suggestion.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another point would be to also add a controller level e.g., to sum it up:

  • application
  • plugin (if part of the request)
  • controller (prefixed by plugin if part of the request)
  • action (prefixed by plugin if part of the request, prefixed by controller)

... each... top and bottom.
It is then to the implementer if to use a more global or more decoupled approach (more requests, smaller files) and where to strike the balance. It allows to defer most/all css/js after the dom is loaded.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The platform stylesheet and script are auto-built by AssetCompress.

The rest of the suggestion are nice, bit heavy, but nice. And yes, they look fitting into a separate plugin, not even an RFC (if you ask me). Something that people could use in this application template or the official one or any other.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I ll let you know if I have a working copy - maybe you or someone in the community can help me with test cases (by help I mean, help me write them myself).


</head>

<?= $this->fetch('body', '<body' . $this->fetch('bodyAttributes') . '>') ?>

<?= $this->fetch('content'); ?>

<footer id="footer" class="footer">

<?= $this->fetch(
'footer',
sprintf(
'&copy;%s %s',
date('Y'),
read('App.title', env('HTTP_HOST'))
)
);
?>

</footer>

<?= $this->fetch('content') ?>
<?= $this->fetch('footer') ?>
<?= $this->AssetCompress->script('platform') ?>
<?= $this->fetch('script'); ?>

Expand Down
36 changes: 0 additions & 36 deletions src/View/AppView.php
Original file line number Diff line number Diff line change
@@ -1,43 +1,7 @@
<?php
namespace App\View;

use Cake\View\View;

class AppView extends View
{
/**
* {@inheritdoc}
*
* @return void
*/
public function initialize()
{
$this->_setupAssetCompress();
$this->_setupBootstrapUI();
}

/**
* Load the AssetCompress helper.
*
* @return void
*/
protected function _setupAssetCompress()
{
$this->loadHelper('AssetCompress.AssetCompress');
}

/**
* Load the BootstrapUI helpers.
*
* @return void
*/
protected function _setupBootstrapUI()
{
$helpers = ['Flash', 'Form'];
foreach ($helpers as $helper) {
$this->loadHelper($helper, [
'className' => 'BootstrapUI.' . $helper,
]);
}
}
}