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

[FEATURE] Context view helpers #735

Conversation

BenjaminBeck
Copy link
Contributor

With this view-helpers you can use the TYPO3_CONTEXT to output different content for your development,testing or production environment.

Please don´t merge until the following questions are solved:

  1. TYPO3_CONTEXT got added in 6.2 but vhs emconf says 6.0 is supported - how do we deal with that old versions? Can we have view-helpers that only work with 6.2?
  2. I created this view-helpers in the namespace ViewHelpers/Context/*. I used the wording "Context" because the core uses it, although "Environment" would feel more intuitive to me. There are also already some view-helpers in a similar namespace: ViewHelpers/Condition/Context/*. I am open for suggestions to improve this situation.

* GNU General Public License for more details.
* This copyright notice MUST APPEAR in all copies of the script!
* ************************************************************* */
use TYPO3\CMS\Fluid\Core\ViewHelper\AbstractViewHelper;
Copy link
Member

Choose a reason for hiding this comment

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

Missing line break here between file header and imports; here and in following classes. Note to self: needs a php code sniffer sniff!

@NamelessCoder
Copy link
Member

Hi Benjamin - very good stuff, these definitely have a use! Apart from the code style things I noted inline, I'd like the isDevelopment etc. ViewHelpers to reside in v:condition.context.isDevelopment, v:condition.context.isTesting and v:condition.context.isProduction - we've been collecting all the condition ViewHelpers there to make it completely obvious which types of conditions you'll have available with VHS.

@NamelessCoder
Copy link
Member

PS: don't mind the failing Travis build, your code looks fine ;)

@NamelessCoder
Copy link
Member

Sorry, missed your Q regarding minimum TYPO3 version - we already have a couple of those and are planning to make 6.2 the minimum requirement in a very short time also for VHS, so this is not an issue.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 2f1afde on BenjaminBeck:FEATURE_Context_view_helpers into e5aa4d0 on FluidTYPO3:development.

@bjo3rnf
Copy link
Contributor

bjo3rnf commented Dec 10, 2014

👍

@BenjaminBeck
Copy link
Contributor Author

Hi NamelessCoder, thank for reviewing. The issues are corrected now. I will squash the commits once everything is fine.
I also have some unit-tests for this view-helpers, should i put them in this PR or make a separate one?

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.16%) when pulling 1d35989 on BenjaminBeck:FEATURE_Context_view_helpers into b397c2c on FluidTYPO3:development.

@NamelessCoder
Copy link
Member

I will squash the commits once everything is fine.

Perfect, we like squash ;)

I also have some unit-tests for this view-helpers, should i put them in this PR or make a separate one?

Unit tests would be the icing on the cake! If you add those before you squash, A+ :)

@NamelessCoder
Copy link
Member

This is an automated comment based on an automated formal commit review.

Your pull request contains formal errors. Comments have been assigned to each commit in your pull request - please review and adjust. Feel free to ask for help if you need it!

/**
* @test
*/
public function renderCompleteContext (){
Copy link
Member

Choose a reason for hiding this comment

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

Expected 1 space after closing parenthesis; found 0

Copy link
Member

Choose a reason for hiding this comment

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

Expected 1 space after closing parenthesis; found 0

Copy link
Member

Choose a reason for hiding this comment

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

Expected 1 space after closing parenthesis; found 0

@NamelessCoder
Copy link
Member

This is an automated comment based on an automated formal commit review.

Your pull request contains formal errors. Comments have been assigned to each commit in your pull request - please review and adjust. Feel free to ask for help if you need it!

1 similar comment
@NamelessCoder
Copy link
Member

This is an automated comment based on an automated formal commit review.

Your pull request contains formal errors. Comments have been assigned to each commit in your pull request - please review and adjust. Feel free to ask for help if you need it!

@coveralls
Copy link

Coverage Status

Coverage increased (+0.19%) when pulling ba231c3 on BenjaminBeck:FEATURE_Context_view_helpers into b397c2c on FluidTYPO3:development.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.19%) when pulling ba231c3 on BenjaminBeck:FEATURE_Context_view_helpers into b397c2c on FluidTYPO3:development.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) when pulling bacde17 on BenjaminBeck:FEATURE_Context_view_helpers into 4e0f276 on FluidTYPO3:development.

@BenjaminBeck
Copy link
Contributor Author

From my side it´s ready to merge.

NamelessCoder added a commit that referenced this pull request Dec 12, 2014
@NamelessCoder NamelessCoder merged commit 586feac into FluidTYPO3:development Dec 12, 2014
@BenjaminBeck BenjaminBeck deleted the FEATURE_Context_view_helpers branch December 12, 2014 17:45
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