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
Single registered article guest view #9865
Conversation
Update from upstream
I have tested this item 🔴 unsuccessfully on 030de77 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
@@ -133,7 +133,14 @@ | |||
<?php endif; ?> | |||
<?php // Optional teaser intro text for guests ?> | |||
<?php elseif ($params->get('show_noauth') == true && $user->get('guest')) : ?> | |||
<?php echo $this->item->introtext; ?> | |||
<?php echo JLayoutHelper::render('joomla.content.intro_image', $this->item); ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move down below the "if"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, sorry, I was wrong on this: I mistook the if below as the if on "image_intro", but that's already in the JLayout
proposed some changes |
This PR has received new commits. CC: @Webdongle This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
Updated to remove double votes |
Works for me. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
I have tested this item ✅ successfully on dc144b3 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
@rgmears Please mark test result on https://issues.joomla.org/tracker/joomla-cms/9865 |
I don't know how to do that. Sorry. This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
@rgmears @wojsmol its ok I manually added his test :) This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
I cannot test now (on a train with no test environ with me) but by simple code review it is evident that lines 137-140 are a duplicate of line 87. Both echoes the content generated by "afterDisplayTitle" plugins (probably a very seldom used event, but anyway...) which in case will be duplicated... In Joomla events list (https://docs.joomla.org/Plugin/Events/Content) I don't find "afterDisplayTitle": is it the same as "onContentAfterTitle"? |
In https://docs.joomla.org/Plugin/Events/Content a note in the "onContentAfterTitle" event description says that
but I'm unsure of what this is or could be as no further specification is given. What is evident from code is that content generated by this event is displayed only if the "show_intro" param is |
Confirmed by testing: Without:With:The "Test Aftertitle" plugin is:
|
I have tested this item 🔴 unsuccessfully on dc144b3 This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
This solves the duplication of the output for the "onContentAfterTitle" event and also solves the issues that the "onContentPrepare" event is not handled for the intro text. I'm unsure, anyway, if it is correct to fix the latter here or if it would be better to fix it at the "view" level (_so that all templates, even overridden ones_) could benefit of the fix
Update default.php
This PR has received new commits. CC: @rgmears, @smz, @Webdongle, @woluweb This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
As the patch has been updated because of the fail when tested with the edited plugin ... should the Testing Instructions be updated to take into account that part of the test ?
Has me totally confused and am unsure how to perform the test with the edited plugin ? This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
@Webdongle As far as regards testing instructions the last word is of course @brianteeman's one, but I can explain what I did in the PR I proposed to Brian and he later incorporated, if this can help.
This second change requires of course also testing that those directives are indeed honoured (as they are, from my testing). About this second change I'm also unsure (and I expressed my doubts directly to Brian) if it is correct to do that here or it would be better to perform that at an higher level (at the view level, probably) so that not only this template, but also all other templates using the "intro text" would benefit of the fix. Unsure if off topic (about the "onContentAfterTitle" event and the "Show Intro Text" option).The "Show intro text" option seems to be quite a strange beast. The description is:
It indeed does that (intro text not displayed in the "Single Article" view if set to "Hide") but it also controls (with inverted logic) weather the content generated by the "onContentAfterTitle" should be displayed or not. This is applied, apparently in an inconsistent way, throughout different views. For this, I think we'll need the memory of "old timers" to explain why this is done this way and what's the logic behind it (I'm calling @infograf768 for that, as apparently he did put his hands on that and probably should know...) |
I have tested this item ✅ successfully on bc7d64c This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
I have tested this item ✅ successfully on bc7d64c This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
I have tested this item ✅ successfully. Intro Image shows.
|
Tested successfully, This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
Patch tested succesfully after patch intro image shows up as expected This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
I have tested this item ✅ successfully on bc7d64c This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/9865. |
Forgive the uninformed question, but, what does RTC mean? |
Ready To Commit @brianteeman |
Thanks @smz . |
Create a new issue please On 15 April 2016 at 15:04, Sergio Manzi notifications@github.com wrote:
Brian Teeman |
about what? |
This was hackish as it would had fixed things only for this template. Not needed anymore now that the problem is fixed in the view.
... pinging all who were involved in this as I amended it in #9964: thanks for testing and/or review. |
Pull Request for Issue #9830
Testing Instructions
Create a single article with an intro and full image and a readmore
Set the article to registered only
Create a menu link to the article of type single article
In the menu options make sure show unauthorised links is set to show
Now check the menu item on the front end - you should see the introtext but not the intro image and when you login you see all the text and the full image
Apply the patch
Now check the menu item on the front end - you should see the introtext and the intro image and when you login you see all the text and the full image