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 Undefined index: JHtmlBootstrap::startAccordion / JHtmlBootstrap::startTabSet #18573

Merged
merged 2 commits into from
Jan 3, 2018

Conversation

Quy
Copy link
Contributor

@Quy Quy commented Nov 14, 2017

Pull Request for Issue #17853.

Summary of Changes

Contact links section does not have the accordion/tab initialization code so when it is the first section to display, the slide/tab cannot be added to.

Testing Instructions

Install Test English Sample Data demo.
Enable Maximum for Error Reporting.
Under Configuration > Contacts > Contact, set Contact Information to hide.
On front end, click Featured Contacts.
Click Shop Address contact.

Expected result

No notice warnings.

Actual result

Notice: Undefined index: JHtmlBootstrap::startAccordion in C:\xampp\htdocs\joomla-cms\libraries\cms\html\bootstrap.php on line 693

Notice: Undefined index: JHtmlBootstrap::startAccordion in C:\xampp\htdocs\joomla-cms\libraries\cms\html\bootstrap.php on line 694

Notice: Undefined index: JHtmlBootstrap::startAccordion in C:\xampp\htdocs\joomla-cms\libraries\cms\html\bootstrap.php on line 695

or

Notice: Undefined index: JHtmlBootstrap::startTabSet in C:\xampp\htdocs\joomla-cms\libraries\cms\html\bootstrap.php on line 788

Documentation Changes Required

None

@C-Lodder
Copy link
Member

If the following doesn't cause any errors, it should be used instead:

<?php if ($presentation_style === 'sliders') : ?>
	<?php if (!$accordionStarted) : ?>
		<?php echo JHtml::_('bootstrap.startAccordion', 'slide-contact', array('active' => 'display-links')); ?>
		<?php $accordionStarted = true; ?>
	<?php endif; ?>
<?php elseif ($presentation_style === 'tabs') : ?>
	<?php if (!$tabSetStarted) : ?>
		<?php echo JHtml::_('bootstrap.startTabSet', 'myTab', array('active' => 'display-links')); ?>
		<?php $tabSetStarted = true;  ?>
	<?php endif; ?>
<?php endif; ?>

@ladyjer
Copy link
Contributor

ladyjer commented Dec 5, 2017

I have tested this item ✅ successfully on 5a06904

@C-Lodder version works too. CS with ':' should be preferred, but CS with '{' is used everywhere in default.php for second level if statements.
I don't know which one is then best.


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

@ladyjer
Copy link
Contributor

ladyjer commented Dec 5, 2017

@C-Lodder version works too. CS with ':' should be preferred, but CS with '{' is used in this default.php for every second level if.
I don't know which one of the two is the best.


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

@Quy
Copy link
Contributor Author

Quy commented Dec 5, 2017

Per Joomla's Coding Standards, read Mixed language usage (e.g. at the layout files). Be sure to scroll up a little bit.

@Quy
Copy link
Contributor Author

Quy commented Dec 23, 2017

@franz-wohlkoenig can you test please?

@ghost
Copy link

ghost commented Dec 23, 2017

I have tested this item ✅ successfully on 5a06904


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

@ghost
Copy link

ghost commented Dec 23, 2017

Ready to Commit after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Dec 23, 2017
@mbabker mbabker added this to the Joomla 3.8.4 milestone Jan 3, 2018
@mbabker mbabker merged commit 4660a36 into joomla:staging Jan 3, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jan 3, 2018
@Quy Quy deleted the patch-5 branch January 3, 2018 00: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.

None yet

5 participants