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

Minor improvements #17

wants to merge 34 commits into from

Conversation

jadb
Copy link
Member

@jadb jadb commented Feb 20, 2016

No description provided.

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.

<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).

@jadb
Copy link
Member Author

jadb commented Feb 21, 2016

@ionas still playing with things. But ya, am leaning towards including more instead of less and deleting what isnt needed on a per app basis instead of adding on a per app basis. Easier to delete stuff than have to set it up ;)

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

Successfully merging this pull request may close these issues.

3 participants