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

Jump to bottom

Improve script/stylesheet inclusion method (Trac #2210) #2210

Closed
elgg-gitbot opened this issue Feb 16, 2013 · 18 comments
Closed

Improve script/stylesheet inclusion method (Trac #2210) #2210

elgg-gitbot opened this issue Feb 16, 2013 · 18 comments
Labels
engine
Milestone
Elgg 1.8.0

Comments

@elgg-gitbot
Copy link

Original ticket http://trac.elgg.org/ticket/2210 on 40381508-08-15 by ewinslow, assigned to cash.

Elgg version: 1.7

Right now there is only a "metatags" view which is generally used for outputting everything except tags.

I propose that the tag take on the following form...

page_elements/head:

head/metas <title>...</title> head/links head/scripts metatags //for backwards compatibility

This format:

  1. Provides more granular control over views (so now plugin authors can, for example, control how jquery(-ui) is included
  2. Is more semantically suggestive. If you want to insert a script into the head, you would extend head/scripts (makes sense) rather than metatags (huh?).
  3. Allows plugin authors to follow best practices by always including stylesheets before scripts.
@elgg-gitbot
Copy link
Author

ewinslow wrote on 40381511-08-31

I keep forgetting that trac is weird. Here's that layout again:

page_elements/head:
<head>
    head/metas 
    <title>...</title> 
    head/links 
    head/scripts 
    metatags //for backwards compatibility
</head>

@elgg-gitbot
Copy link
Author

ewinslow wrote on 40477822-06-26

This can be done in a completely backwards compatible way and would be oh-so-useful so I think it would be beneficial to squeeze it into 1.7, rather than having to wait 3 months for 1.8 to roll around.

@elgg-gitbot
Copy link
Author

Milestone changed to Elgg 1.7.2 by ewinslow on 40477822-06-26

@elgg-gitbot
Copy link
Author

Attachment added by ewinslow on 40481177-03-07: 2210_granular_head_view.diff

@elgg-gitbot
Copy link
Author

ewinslow wrote on 40481181-12-19

I've added a head/title view as well to allow plugins to override how the title is displayed, e.g. to put their site name after the page name, rather than before.

@elgg-gitbot
Copy link
Author

brettp wrote on 40522508-09-04

Is there any urgency to put this in the 1.7.2 release or can it be delayed to 1.8?

@elgg-gitbot
Copy link
Author

ewinslow wrote on 40522559-03-10

It can be delayed, but I don't necessarily see why we would? It's a very easy fix and does not disrupt current themes -- or are you not totally convinced of that?

@elgg-gitbot
Copy link
Author

brettp wrote on 40522624-12-10

I'm not totally convinced of that :)

The approach definitely looks solid with fallbacks to page_elements/header, and it's an improvement to what's there, but people are sometimes highly creative with how they do their themes and get rightfully cranky when we break that.

Let me do a bit more investigating to how some of the popular themes work with header.php...

@elgg-gitbot
Copy link
Author

cash wrote on 40524842-06-04

In 1.7.1 we squeezed in a change to friendly titles and time that someone really wanted included. I ended up having to rewrite it for 1.7.2 because it broke something that we didn't foresee. Because the change was made so close to the release, we did not get enough testing to notice the problem. I'm not saying that is the case this time but would rather error on the side of caution.

I'd also like to compare the advantages of making the js and css inclusions a view as opposed to adding a set of registration functions similar to this: http://github.com/cash/manage_externals. If we introduce a view for js in 1.7.2 and end up using registration functions in 1.8, it causes headaches for developers.

@elgg-gitbot
Copy link
Author

ewinslow wrote on 40525421-04-25

Sounds like a plan. I like the registration idea even better.

@elgg-gitbot
Copy link
Author

ewinslow wrote on 40526408-08-10

Since the consensus seems to be to wait until 1.8 for this (or do something else entirely), I'm changing the milestone.

@elgg-gitbot
Copy link
Author

Milestone changed to Elgg 1.8 by ewinslow on 40526408-08-10

@elgg-gitbot
Copy link
Author

ewinslow wrote on 40832247-03-06

Let's go with the registration functions

@elgg-gitbot
Copy link
Author

Title changed from Make head view more granular to Improve script/stylesheet inclusion method by ewinslow on 40832247-03-06

@elgg-gitbot
Copy link
Author

cash wrote on 40832255-04-16

I was thinking it would be good to give people the option on whether they wanted their external javascript included in the head or footer. Besides that, I think the functions I put together handle this.

@elgg-gitbot
Copy link
Author

ewinslow wrote on 40832268-07-26

I'll let you take care of this one then

@elgg-gitbot
Copy link
Author

cash wrote on 40832688-04-25

(In [svn:7169]) Refs #2210 - added functions for managing javascript and css files

@elgg-gitbot
Copy link
Author

cash wrote on 40843429-03-08

(In [svn:7233]) Fixes #2210 - using new functions for registering the core js and css files

@elgg-gitbot elgg-gitbot mentioned this issue Feb 16, 2013
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
engine
Milestone
Elgg 1.8.0
Development

No branches or pull requests
1 participant
@elgg-gitbot