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

Default values for sections. #36

Merged
merged 1 commit into from
Oct 20, 2014
Merged

Default values for sections. #36

merged 1 commit into from
Oct 20, 2014

Conversation

vensko
Copy link

@vensko vensko commented Oct 19, 2014

No description provided.

@reinink
Copy link
Contributor

reinink commented Oct 20, 2014

Hey @vensko, thanks so much for taking the time to contribute to Plates! Very much appreciated. While I like the problem you're trying to solve here, I'm concerned that it causes repeating logic. Consider this example:

<?php if ($this->section('sidebar', $this->fetch('default-sidebar'))): ?>
    <div id="sidebar">
        <?=$this->section('sidebar', $this->fetch('default-sidebar'))?>
    </div>
<?php endif ?>

Currently you would do this instead:

<div id="sidebar">
    <?php if ($this->section('sidebar')): ?>
        <?=$this->section('sidebar')?>
    <?php else ?>
        <?=$this->fetch('default-sidebar')?>
    <?php endif ?>
</div>

Now I understand the current approach requires a couple extra lines of code (mostly caused by the if statement), but I do find this much more readable.

You could assign the response to a variable, but that technically breaks our syntax recommendation to "avoid variable assignment".

<?php if ($sidebar = $this->section('sidebar', $this->fetch('default-sidebar'))): ?>
    <div id="sidebar">
        <?=$sidebar?>
    </div>
<?php endif ?>

Finally, I'm a little concerned that this approach encourages putting HTML into PHP strings, which I find generally makes templates less readable. Clearly this doesn't have to be done like this (consider my fetch() examples above), but unfortunately I think it probably will more often than not.

Because of these concerns, I'm thinking that the current if/else approach, albeit a little less elegant, may still be more appropriate. I do this this will prompt me to add a "default section content" example to the documentation though.

Either way, thanks again for contributing to this project! I hope the above reasons make sense.

@reinink reinink closed this Oct 20, 2014
@vensko
Copy link
Author

vensko commented Oct 20, 2014

This feature is meant to be used with inline text, for instance:
<title><?=$this->section('pagetitle', 'Site name')?></title>
Also, it makes transition from Symfony Templating with Slots Helper easier.

@reinink
Copy link
Contributor

reinink commented Oct 20, 2014

Okay, I hadn't considered that use case. I guess there's no reason why both approaches couldn't be used.

@reinink reinink reopened this Oct 20, 2014
reinink added a commit that referenced this pull request Oct 20, 2014
Default values for sections.
@reinink reinink merged commit 104cc53 into thephpleague:master Oct 20, 2014
@reinink
Copy link
Contributor

reinink commented Oct 20, 2014

This feature has been added to the 3.0.3 release.

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.

4 participants