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

[4.0] Remove margin between accordion slides #29579

Merged
merged 2 commits into from
Jun 12, 2020
Merged

[4.0] Remove margin between accordion slides #29579

merged 2 commits into from
Jun 12, 2020

Conversation

SharkyKZ
Copy link
Contributor

Fixes #29311.

Summary of Changes

Removes margin between Bootstrap accordion slides.

Testing Instructions

Edit a module in frontend.

Expected result

Looks OK.

Actual result

Gap between slides and first slide is missing the bottom border:

Documentation Changes Required

No.

@infograf768
Copy link
Member

infograf768 commented Jun 12, 2020

I have a B/C problem here with com_localise after this patch.
I do use the below code and display a table in the slide

<?php echo HTMLHelper::_('bootstrap.startAccordion', 'slide-translations', array('active' => '')); ?>
<?php echo HTMLHelper::_('bootstrap.addSlide', 'slide-translations', Text::_('COM_LOCALISE_SLIDER_TRANSLATIONS_LEGEND'), 'legend'); ?>

Then I have another accordeon + slide.
I lose the mb-2 margin

.mb-2, .my-2 {
    margin-bottom: 0.5rem !important;
}

Before this PR

Screen Shot 2020-06-12 at 10 22 29

After this PR
Screen Shot 2020-06-12 at 10 22 56

@infograf768
Copy link
Member

infograf768 commented Jun 12, 2020

I can evidently solve it by adding the mb-2 class to the div enclosing this template but I wonder if this PR is the correct solution.

		<div class="col-md-12 mb-2">
			<?php echo $this->loadTemplate('legend'); ?>
		</div>

@ceford
Copy link
Contributor

ceford commented Jun 12, 2020

I have tested this item ✅ successfully on 878e3bc

First bottom margin present - but no gaps between unopened tabs, is that intended.


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

@infograf768
Copy link
Member

I have tested this item ✅ successfully on 878e3bc

I guess will just have to modify my component.


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

@infograf768
Copy link
Member

rtc


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 12, 2020
@richard67 richard67 merged commit 029fde2 into joomla:4.0-dev Jun 12, 2020
@richard67
Copy link
Member

Thanks all!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 12, 2020
@richard67 richard67 added this to the Joomla 4.0 milestone Jun 12, 2020
@SharkyKZ SharkyKZ deleted the j4/markup/accordion branch June 12, 2020 11:05
@SharkyKZ
Copy link
Contributor Author

@ceford Yes, that's expected.

@infograf768 you can also pass mb-2 class to HTMLHelper::_('bootstrap.addSlide').

This PR is fine in that it fixes the default styling. Before patch, this looked fine in Atum because it has custom CSS but not in Cassiopeia or in any stock Bootstrap template.

sakiss pushed a commit to sakiss/joomla-cms that referenced this pull request Oct 16, 2020
* Remove margin between accordion slides

* Add margin to field
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.

5 participants