Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[WebProfilerBundle] Fixed icon alignment issue using Bootstrap 4.1.2 #27941

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

Merged
merged 1 commit into from
Jul 13, 2018
Merged

[WebProfilerBundle] Fixed icon alignment issue using Bootstrap 4.1.2 #27941

merged 1 commit into from
Jul 13, 2018

Conversation

jmsche
Copy link
Contributor

@jmsche jmsche commented Jul 13, 2018

Q A
Branch? 2.8
Bug fix? yes?
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets N/A
License MIT
Doc PR N/A

Hi,

This PR fixes an issue introduced by Bootstrap 4.1.2: it vertically aligns SVG icons in the middle instead of letting it stay at the baseline.

I'm not sure this PR is relevant but, if I'm not the only Bootstrap user, I guess it will be useful to many and it does not break anything: it enforces what should be default.

[Update] Here is the related PR merged into Bootstrap that causes the issue: twbs/bootstrap#25874

[Update 2] Before the fix:

fix_before

After the fix:

fix_after

@fabpot
Copy link
Member

fabpot commented Jul 13, 2018

Thank you @jmsche.

@fabpot fabpot merged commit ecef6f1 into symfony:2.8 Jul 13, 2018
fabpot added a commit that referenced this pull request Jul 13, 2018
…trap 4.1.2 (jmsche)

This PR was merged into the 2.8 branch.

Discussion
----------

[WebProfilerBundle] Fixed icon alignment issue using Bootstrap 4.1.2

| Q             | A
| ------------- | ---
| Branch?       | 2.8
| Bug fix?      | yes?
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | N/A
| License       | MIT
| Doc PR        | N/A

Hi,

This PR fixes an issue introduced by Bootstrap 4.1.2: it vertically aligns SVG icons in the middle instead of letting it stay at the baseline.

I'm not sure this PR is relevant but, if I'm not the only Bootstrap user, I guess it will be useful to many and it does not break anything: it enforces what should be default.

[Update] Here is the related PR merged into Bootstrap that causes the issue: twbs/bootstrap#25874

[Update 2] Before the fix:

![fix_before](https://user-images.githubusercontent.com/3929498/42696806-966cfc9e-86b9-11e8-90a9-7a6dc18a1809.png)

After the fix:

![fix_after](https://user-images.githubusercontent.com/3929498/42696821-9df8ef22-86b9-11e8-8c6c-62a4afa752a3.png)

Commits
-------

ecef6f1 [WebProfilerBundle] Fixed icon alignment issue using Bootstrap 4.1.2
@@ -263,6 +263,7 @@ div.sf-toolbar .sf-toolbar-block a:hover {
border-width: 0;
position: relative;
top: 8px;
vertical-align: baseline;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it actually also apply to images ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that shouldn't be an issue as the same is already applied to .sf-toolbarreset *

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For me, the css rule already exist in .sf-toolbarreset * selector. It's not necessary to duplicate this rule. Just improve it.

Add !important and that's ok. No ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.