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

Fix showon for frontend #15202

Merged
merged 1 commit into from Apr 10, 2017
Merged

Fix showon for frontend #15202

merged 1 commit into from Apr 10, 2017

Conversation

Bakual
Copy link
Contributor

@Bakual Bakual commented Apr 10, 2017

Pull Request for Issue #15183 .

Summary of Changes

Taking care of the case where we have no formcontrol but a group.

Testing Instructions

  • Try showon in frontend template editing (eg Protostar: "Google Font for Headings")
  • Try showon in various other places

Expected result

Showon works as expected in all places

Actual result

Showon doesn't work in frontend template editing

Documentation Changes Required

None

@brianteeman
Copy link
Contributor

I have tested this item ✅ successfully on a12fd37


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15202.

@AlexRed
Copy link
Contributor

AlexRed commented Apr 10, 2017

ok for Protostar: "Google Font for Headings"
but in frontend "Site Settings" --> SEO Settings --> Search Engine Friendly URLs
if yes don't show the other Seo settings as in Global configuration (Use URL Rewriting, Adds Suffix to URL, Unicode Aliases)
it should?


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15202.

@brianteeman
Copy link
Contributor

@AlexRed the fields you are talking about do not exist in that form so that form is working as intended. there is no showon stuff in that form https://github.com/joomla/joomla-cms/blob/staging/components/com_config/model/form/config.xml So if you feel that those fields should exist in that view it is a different issue to showon that this PR fixes

@AlexRed
Copy link
Contributor

AlexRed commented Apr 10, 2017

ok, sorry

@AlexRed
Copy link
Contributor

AlexRed commented Apr 10, 2017

I have tested this item ✅ successfully on a12fd37

Patch ok for me.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/15202.

@Bakual
Copy link
Contributor Author

Bakual commented Apr 10, 2017

@brianteeman is right here in that this is the main reason it doesn't work 😄
However even if you do add the missing fields, it would still not work,
You would have to change those lines

<?php foreach ($this->form->getFieldset('seo') as $field) : ?>
	<div class="control-group">
		<div class="control-label"><?php echo $field->label; ?></div>
		<div class="controls"><?php echo $field->input; ?></div>
	</div>
<?php endforeach; ?>

(https://github.com/joomla/joomla-cms/blob/staging/components/com_config/view/config/tmpl/default_seo.php#L14-L23)
to the single line
<?php echo $this->form->renderFieldset('seo'); ?>

@ghost
Copy link

ghost commented Apr 10, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Apr 10, 2017
@wilsonge
Copy link
Contributor

Just on a quick code review and no more (so might be talking rubbish) - is it possible to have a form control and group with the same name, in that case would this start to cause issues?

@Bakual
Copy link
Contributor Author

Bakual commented Apr 10, 2017

Just on a quick code review and no more (so might be talking rubbish) - is it possible to have a form control and group with the same name, in that case would this start to cause issues?

The formcontrol usually is just "jform". If we have both the group and form with the same name, it should still work but if we have one form without groups and another one without formcontrol and they have the same names, then it may become an issue.
In theory, it may be possible. But I doubt they would appear on the same page in practice. Also there will be likely other issues as well.
Usually we only have one form on a page so this should not become an issue.

@rdeutz rdeutz added this to the Joomla 3.7.0 milestone Apr 10, 2017
@rdeutz rdeutz merged commit 5f793b2 into joomla:staging Apr 10, 2017
@joomla-cms-bot joomla-cms-bot added PR-staging and removed RTC This Pull Request is Ready To Commit labels Apr 10, 2017
@Bakual Bakual deleted the FixShowonFrontend branch April 10, 2017 15:53
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.

None yet

6 participants