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

Solo Grid Margin Issue #6685

Closed
mattdemps opened this Issue Oct 31, 2013 · 5 comments

Comments

Projects
None yet
4 participants
@mattdemps

mattdemps commented Oct 31, 2013

When using a solo grid you get extra margin on either end causing it to have a slightly off center layout.

I have noticed this in 1.4.0 rc1, though it could be an issue in past versions.

JSBIN Here:
http://jsbin.com/OXElOlu/1/

@agcolom

This comment has been minimized.

Show comment
Hide comment
@agcolom

agcolom Oct 31, 2013

Member

To confirm: here's the simplified version: http://jsbin.com/uLIxoGU/3 (removed all unrelated CSS and JavaScript). The same with 1.3.2 does not show the pb: http://jsbin.com/uLIxoGU/2
The same with 1.4.0-beta.1 does not show the pb either: http://jsbin.com/uLIxoGU/4

This seems to be due to
.ui-grid-solo > [class*="ui-"] {
margin-right: .3125em;
margin-left: .3125em;
}

@uGoMobi This is the commit: 71448a6#diff-73214b44ef91605e3cf8574cad62e419R54

Member

agcolom commented Oct 31, 2013

To confirm: here's the simplified version: http://jsbin.com/uLIxoGU/3 (removed all unrelated CSS and JavaScript). The same with 1.3.2 does not show the pb: http://jsbin.com/uLIxoGU/2
The same with 1.4.0-beta.1 does not show the pb either: http://jsbin.com/uLIxoGU/4

This seems to be due to
.ui-grid-solo > [class*="ui-"] {
margin-right: .3125em;
margin-left: .3125em;
}

@uGoMobi This is the commit: 71448a6#diff-73214b44ef91605e3cf8574cad62e419R54

@mattdemps

This comment has been minimized.

Show comment
Hide comment
@mattdemps

mattdemps Oct 31, 2013

@agcolom

Thanks for digging into this issue and finding the commit for traceability. I know there are other selectors using these margin rules:

Full Selector Stack:
[class_=ui-block-]>.ui-btn, [class_=ui-block-]>.ui-select, [class_=ui-block-]>.ui-checkbox, [class_=ui-block-]>.ui-radio, [class_=ui-block-]>button.ui-btn-inline, [class_=ui-block-]>button.ui-btn-icon-notext, .ui-grid-solo>[class*=ui-]

As long as there are no negative implications to removing the .ui-grid-solo > [class*="ui-"] selector, I vote to to remove it, its addition may have been a mistake.

mattdemps commented Oct 31, 2013

@agcolom

Thanks for digging into this issue and finding the commit for traceability. I know there are other selectors using these margin rules:

Full Selector Stack:
[class_=ui-block-]>.ui-btn, [class_=ui-block-]>.ui-select, [class_=ui-block-]>.ui-checkbox, [class_=ui-block-]>.ui-radio, [class_=ui-block-]>button.ui-btn-inline, [class_=ui-block-]>button.ui-btn-icon-notext, .ui-grid-solo>[class*=ui-]

As long as there are no negative implications to removing the .ui-grid-solo > [class*="ui-"] selector, I vote to to remove it, its addition may have been a mistake.

@ghost ghost assigned jaspermdegroot Nov 1, 2013

@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Nov 1, 2013

Member

.ui-grid-solo > [class*="ui-"] was added with commit 71448a6 to fix #6643. I now realize that #6643 wasn't an issue in the framework CSS, but an issue with the demo that was used to illustrate the problem. There we don't wrap the button inside ui-grid-solo in an element with ui-block-a.

I will fix that demo and revert 71448a6. We don't make changes in the framework at the moment because we are on RC and I don't find this bug critical enough to release another RC. Until 1.4.1 it can be solved by negating the margin in your custom CSS. See http://jsbin.com/AKOmUxa/77/edit. Sorry for the inconvenience!

Member

jaspermdegroot commented Nov 1, 2013

.ui-grid-solo > [class*="ui-"] was added with commit 71448a6 to fix #6643. I now realize that #6643 wasn't an issue in the framework CSS, but an issue with the demo that was used to illustrate the problem. There we don't wrap the button inside ui-grid-solo in an element with ui-block-a.

I will fix that demo and revert 71448a6. We don't make changes in the framework at the moment because we are on RC and I don't find this bug critical enough to release another RC. Until 1.4.1 it can be solved by negating the margin in your custom CSS. See http://jsbin.com/AKOmUxa/77/edit. Sorry for the inconvenience!

jaspermdegroot added a commit that referenced this issue Nov 1, 2013

Demos: Buttons inside grid solo should be wrapped in element with ui-…
…block-a

This is the correct fix for #6643. The changes from commit 71448a6 have
to be reverted. See
#6685 (comment)
2

jaspermdegroot added a commit that referenced this issue Nov 18, 2013

Demos: Buttons inside grid solo should be wrapped in element with ui-…
…block-a

This is the correct fix for #6643. The changes from commit 71448a6 have
to be reverted. See
#6685 (comment)
2
@jaspermdegroot

This comment has been minimized.

Show comment
Hide comment
@jaspermdegroot

jaspermdegroot Dec 1, 2013

Member

When fixing this we have to test navbars with one button (see #6775)

Member

jaspermdegroot commented Dec 1, 2013

When fixing this we have to test navbars with one button (see #6775)

@DaazKu

This comment has been minimized.

Show comment
Hide comment
@DaazKu

DaazKu Dec 2, 2013

@uGoMobi

@uGoMobi

#6775 shows the problem with one and multiples button. Even with 6 button you still get a problem because of the margin-right: -4px;

Disregarding any regression, removing those 2 fix the problem:

/* fixes gaps caused by subpixel problem */
.ui-navbar li:last-child .ui-btn {
margin-right: -4px;
}

[class_="ui-block-"] > .ui-btn,
[class_="ui-block-"] > .ui-select,
[class_="ui-block-"] > .ui-checkbox,
[class_="ui-block-"] > .ui-radio,
[class_="ui-block-"] > button.ui-btn-inline,
[class_="ui-block-"] > button.ui-btn-icon-notext,
.ui-grid-solo > [class*="ui-"] {
margin-right: .3125em;
margin-left: .3125em;
}

DaazKu commented Dec 2, 2013

@uGoMobi

@uGoMobi

#6775 shows the problem with one and multiples button. Even with 6 button you still get a problem because of the margin-right: -4px;

Disregarding any regression, removing those 2 fix the problem:

/* fixes gaps caused by subpixel problem */
.ui-navbar li:last-child .ui-btn {
margin-right: -4px;
}

[class_="ui-block-"] > .ui-btn,
[class_="ui-block-"] > .ui-select,
[class_="ui-block-"] > .ui-checkbox,
[class_="ui-block-"] > .ui-radio,
[class_="ui-block-"] > button.ui-btn-inline,
[class_="ui-block-"] > button.ui-btn-icon-notext,
.ui-grid-solo > [class*="ui-"] {
margin-right: .3125em;
margin-left: .3125em;
}

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