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

[5.2] remove margin-bottom on .btn styling to prevent nasty offset #43247

Open
wants to merge 13 commits into
base: 5.2-dev
Choose a base branch
from

Conversation

hans2103
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

This PR will remove the following piece of SCSS and uses proper Bootstrap classNames to achieve the same goal.

@include media-breakpoint-down(lg) {
.btn {
margin-bottom: .25rem;
}
.input-group .btn {
margin-bottom: 0;
}
}

PR #32697 introduced a margin-bottom of .25em on all buttons.
A fix made by @brianteeman prevented this to happen on buttons in an input-group.
Both are not needed when using proper Bootstrap classNames on the wrapping element.

Testing Instructions

  • Joomla 5.1 with sample data installed.
  • Compile SCSS
  • Head over to page Typography and reduce the viewport. On mobile the elements with className .btn are stacked with a proper gap between.
  • Login on frontend and click an item to edit. On mobile the elements with className .btn are stacked with a proper gap between.
  • Go to a random page to invoke a 404. Inspect the button with home icon and text "Home page". On mobile this button has no obsolete margin-bottom anymore. And all elements with className .btn don't have a obsolete margin-bottom anymore.

Actual result BEFORE applying this Pull Request

Scherm­afbeelding 2024-04-10 om 17 15 10 Scherm­afbeelding 2024-04-10 om 17 13 25 Scherm­afbeelding 2024-04-10 om 17 12 12

Expected result AFTER applying this Pull Request

Scherm­afbeelding 2024-04-10 om 17 15 31 Scherm­afbeelding 2024-04-10 om 17 13 05 Scherm­afbeelding 2024-04-10 om 17 12 32

Comment

Even Bootstrap core has no margins around buttons. See screenshot below where I have disabled the DOCS styling.

Scherm­afbeelding 2024-04-10 om 17 05 01

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.1-dev labels Apr 10, 2024
@brianteeman
Copy link
Contributor

Which was the PR I made a partial fix in? Would like to check exactly what the issue it was fixing is still fixed. Thanks

@hans2103
Copy link
Contributor Author

hans2103 commented Apr 11, 2024

Which was the PR I made a partial fix in? Would like to check exactly what the issue it was fixing is still fixed. Thanks

#33928

Long live git blame for this.

@brianteeman
Copy link
Contributor

Thanks

@HLeithner HLeithner changed the base branch from 5.1-dev to 5.2-dev April 24, 2024 09:06
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 5.2-dev.

@HLeithner HLeithner changed the title [5.1] remove margin-bottom on .btn styling to prevent nasty offset [5.2] remove margin-bottom on .btn styling to prevent nasty offset Apr 24, 2024
@Quy Quy removed the PR-5.1-dev label Apr 25, 2024
@joomla joomla deleted a comment Jun 1, 2024
@JeroenMoolenschot
Copy link
Member

I have tested this item ✅ successfully on bbc2940


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Language Change This is for Translators NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants