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.

Sign up for GitHub

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

Abstract header #74

Closed
cassidyjames opened this issue Jan 12, 2015 · 8 comments
Closed

Abstract header #74

cassidyjames opened this issue Jan 12, 2015 · 8 comments
Milestone
Total Automation

Comments

@cassidyjames
Copy link
Contributor

We should probably pull the header out into its own file, much like the footer in #65.

A simple way to do this while still being able to set page-specific titles/descriptions is to set have some variables in the header, then set them in the pages before including the header. Something like:

<?php
$page_title = 'elementary OS Freya';
$page_description = 'A fast and free replacement for Windows and OS X. Pay what you want or download for free.';

include_once('header.php');
?>

Then in the header you can do this:

<title><?=$page_title;?></title>
<meta name="description" content="<?=$page_description;?>" />

...etc.
@cassidyjames cassidyjames modified the milestone: Lightweight backend Jan 12, 2015
@lewisgoddard
Copy link
Member

  1. Short php tags are generally discouraged because of things like xml starting with <?xml and ending with ?> as well, however I am not sure how that applies to the short echo tags, but I always do <?php echo $thing; ?> for compatability and so I feel safe. We should probably make a decision on that.
  2. include_once isn't a function, it's a control structure, so it should be formatted include_once 'header.php';
  3. I don't see why we'd want to stop the header being included twice, good code should do that. No need for PHP to keep track of it, that's for function declarations and the like.

@martpie
Copy link
Collaborator

martpie commented Jan 12, 2015

I agree on 1. & 2., but not for 3. (or I misuderstood it)

let's imagine something like:

Pages
|- home.php
|- 404.php
|- [...].php

Parts
|- header.php
|- footer.php

In Pages we would have all the content + specific meta of the page (title and description) + 1 include at the top (header) and 1 include at the bottom (footer)

In Parts we would have ... parts !

So:
header.php

<!doctype html>
<html>
    <head>
        <meta charset="UTF-8">
        <meta name="viewport" content="width=device-width, initial-scale=1.0">
        <meta name="author" content="elementary LLC">
        <link rel="shortcut icon" href="favicon.ico">
        <link rel="stylesheet" type="text/css" media="all" href="http://fonts.googleapis.com/css?family=Open+Sans:400,300">
        <link rel="stylesheet" type="text/css" media="all" href="http://maxcdn.bootstrapcdn.com/font-awesome/4.2.0/css/font-awesome.min.css">

home.php

        <meta name="description" content="elementary OS, a fast and open replacement for Windows and OS X">
        <title>elementary OS</title>
</head>

<body>
        <div id="content-container">
                blablabla...
        </div>

footer.php

<footer>
            Copyright &copy; 2015 elementary LLC.
            <ul>
                <li><a href="http://blog.elementaryos.org" target="_blank" title="Tumblr"><i class="fa fa-tumblr"></i></a></li>
                <li><a href="https://twitter.com/elementary" target="_blank" title="Twitter"><i class="fa fa-twitter"></i></a></li>
                <li><a href="http://reddit.com/r/elementaryos" target="_blank" title="Reddit"><i class="fa fa-reddit"></i></a></li>
                <li><a href="https://plus.google.com/114635553671833442612" target="_blank" title="Google+"><i class="fa fa-google-plus"></i></a></li>
                <li><a href="https://github.com/elementary/mvp" target="_blank" title="Github"><i class="fa fa-github"></i></a></li>
                <li><a href="https://www.facebook.com/elementaryos" target="_blank" title="Facebook"><i class="fa fa-facebook"></i></a></li>
            </ul>
        </footer>
    </body>
</html>

@cassidyjames
Copy link
Contributor Author

Yeah, I don't think there's any harm in include_once. :P We definitely wouldn't ever want the header included more than once, so it makes sense.

@KeitIG, I think I prefer keeping the page's HTML to just within the <body> tag. It's a lot easier to make sure all the opening/closing tags line up, and it also keeps the variables up top of each page's file so they can be used in several places within the head.

@cassidyjames
Copy link
Contributor Author

@lewisgoddard you're right about 1 and 2. Thanks for the heads up (I wrote that code without reference on my break).

@lewisgoddard
Copy link
Member

I don't see how you guys disagree with 3.

I would use include 'part.php'; rather than include_once 'part.php';, that's all. We should never be calling include twice on the header anyway, so PHP shouldn't be using memory keeping track of something that should never happen.

@cassidyjames
Copy link
Contributor Author

@lewisgoddard that's fair. If there's a performance benefit for using include instead of include_once I have no qualms.

@lewisgoddard
Copy link
Member

There is a minor memory reduction, gut multiplied across thousands of release-day requests, it stacks up. A good general rule of thumb is to use it to stop functions and classes being re-declared (which is a fatal error), rather than template control.

@cassidyjames
Copy link
Contributor Author

@KeitIG I'm not sure if you've started work on this or not, but for a directory structure, I think this is pretty sensible:

  • index.php
  • 404.php
  • [...any other pages].php
  • images/
    • [image files]
  • styles/
    • [css files]
  • includes/
    • header.php
    • footer.php

Feel free to correct me if I'm wrong. :)

cassidyjames pushed a commit that referenced this issue Jan 12, 2015
Merge pull request #79 from elementary/templates
fb408ee 
Add basic templating. Fix #42, Fix #65, Fix #74
@lewisgoddard lewisgoddard modified the milestones: Lightweight backend, Total Automation Nov 2, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Total Automation
Development

No branches or pull requests
3 participants
@cassidyjames @lewisgoddard @martpie