Navbar visual issue #91

Closed
ksfreitas opened this Issue May 5, 2012 · 37 comments

Comments

Projects
None yet
4 participants
Contributor

ksfreitas commented May 5, 2012

When the browser window width is small, the Navbar jump down. This reaction cannot be simulated in non-gwt bootstrap.

Owner

soundTricker commented May 6, 2012

Thanks for your reporting @ksfreitas.
Do it happen in our showcase or your app?
If it happen in showcase,maybe it's showcase.css's bug.

Contributor

ksfreitas commented May 6, 2012

In Showcase and in my application. Note: I don't use any css of showcase in my application. The problem can be simulated with the code below (tested in Firefox 12, Internet Explorer 9 and Chrome):

public class TestPage implements com.google.gwt.core.client.EntryPoint {

@Override
public void onModuleLoad() {
    Navbar navbar = new Navbar();
    navbar.setPosition(NavbarPosition.TOP);
    RootPanel.get().add(navbar);
}

}

The problem appear when the browser width is less than 980px.

Owner

soundTricker commented May 6, 2012

Thanks :)
OK.
I'll check it.

Contributor

ksfreitas commented May 6, 2012

The problem appear to be in bootstrap-responsive.min.css

When remove the property "position", of @media(max-width:979px) .navbar-fixed-top, the position is corrected, but the controls (logo and menu button) are misaligned...

Owner

soundTricker commented May 6, 2012

Umm...
I think maybe Navbar#setPaddingTop method is bad work.
when browser is changed width,This method is worked.

@dominikmayer , @caarlos0
What is Navbar#setPaddingTop method?
I don't know why we need it.
Please tell me. ;-)

Contributor

ksfreitas commented May 6, 2012

Good news. I fix it changing the CSS (my last post):

.navbar-fixed-top{position:fixed;margin-bottom:18px;margin-left:0px;margin-right:0px;}

Contributor

ksfreitas commented May 6, 2012

It's not perfect yet... Some issues appear... I'll try to fix.

Owner

soundTricker commented May 6, 2012

Ummm....
I can't agree fixing bootstrap's css.
If this bug don't happen in native bootstrap,we have to fix our code.

Contributor

ksfreitas commented May 6, 2012

I agree!

Navbar#setPaddingTop isn't the cause. Some top-margin (42px) is already applied in parent div (id="content"). When the @media is triggered, the "position" change to "static" and triggers this margin. I'm looking where which starts.

Owner

soundTricker commented May 6, 2012

OK.
But why do not native bootstrap happen?
If we fix bootstrap.css,we should know it.

Contributor

ksfreitas commented May 6, 2012

Forgive me (I made a big mess)...

The 42px I said is started in Navbar#setPaddingTop... So, you're right.

:S

Contributor

ksfreitas commented May 6, 2012

Temporary solution:

Navbar navbar = new Navbar() {

        @Override
        protected void setPaddingTop(boolean padding) {
            super.setPaddingTop(false);
        }
    };
Owner

soundTricker commented May 6, 2012

Thanks for your hack. :-)
but I don't know why there is this method.
maybe this method is created by @dominikmayer or @caarlos0 .
I want to ask that what this method means.

Owner

caarlos0 commented May 6, 2012

hmmm.. I think that @dominikmayer wrote this method...
But, I think I know why...

In an old version of bootstrap, we have to set a padding-top in the body because otherwise the container will be back of the menu...
But I believe thats dont necessary anymore...

Owner

soundTricker commented May 6, 2012

hm...
OK.
I'll try removing method.

Owner

caarlos0 commented May 6, 2012

I check it here... this method is needed...

Inspecting the DOM, I found that is something with responsiveness.. in bootstrap showcase, when the width change, the padding-top is override, in our showcase not.. I dont know why...

Owner

caarlos0 commented May 6, 2012

I found the problem, bootstrap-responsive.css is missing in our showcase :(

Owner

soundTricker commented May 6, 2012

um.
Maybe our showcase has one style bug.
The showcase apply below style.

#content {
    padding-top: 60px;
}

Maybe it's not necessary.
but I think we have other bug.

Owner

caarlos0 commented May 6, 2012

I think not.. I think that the problem is the missing css file...

Owner

soundTricker commented May 6, 2012

Right now,bootstrap-responsive.css is inserted as String with style tag same of the past.

Owner

caarlos0 commented May 6, 2012

hmm, yeah, I see it right now.. we have to change it, huh?

Owner

caarlos0 commented May 6, 2012

I can "fix" that.. but, we have a little lag when change from a large screen to a smaller one..

Contributor

ksfreitas commented May 6, 2012

I'm handling now with ResizeHandler, turning on when the size is less or equals than 979.

Owner

dominikmayer commented May 6, 2012

If anyone knows a way to get rid of setPaddingTop please do remove it. I didn't know any other way to fix the spacing issue. The delay is due to a scheduler call that can be removed.

caarlos0 was assigned May 14, 2012

Owner

caarlos0 commented May 14, 2012

From http://twitter.github.com/bootstrap/components.html#navbar

When you affix the navbar, remember to account for the hidden area underneath. Add 40px or more of padding to the <body>. Be sure to add this after the core Bootstrap CSS and before the optional responsive CSS.
Owner

caarlos0 commented May 15, 2012

I just cant understand what's happening here...

@caarlos0 caarlos0 added a commit that referenced this issue May 15, 2012

@caarlos0 caarlos0 I think the problem is the schedule time, I change it from 250 to 10 …
…and it looks good to me. Please, someone test it too. - refs #91
6ff5909
Owner

caarlos0 commented May 15, 2012

@ksfreitas can you take a look at this, and see if it works now, please? thanks

Owner

caarlos0 commented May 19, 2012

navbar looks worst now... what changed?

Owner

soundTricker commented May 19, 2012

How do you look?

Owner

caarlos0 commented May 19, 2012

Screen

Owner

caarlos0 commented May 19, 2012

A app that I am testing right now looks bat too :S

Owner

soundTricker commented May 19, 2012

That's collapse plugin of Twitter Bootstrap.
If display width become less than 767px,Navbar menus be collapse in ≡ icon.
Maybe same as Twitter Bootstarp showcase.

and Top of navbar space is applied showcase.css.
maybe we should remove its css writing.

Owner

caarlos0 commented May 19, 2012

yeah, I was talking about the space :)

Owner

soundTricker commented May 19, 2012

OK. I'll remove it.

Contributor

ksfreitas commented May 19, 2012

Humm... I know what is going on... In showcase, we have a fixed margin with 42px... It must be removed!

Owner

caarlos0 commented May 19, 2012

I believe that I found the problem. I fixed it in my fork. Will do more tests and ASAP i'll push it to upstream master :)

caarlos0 closed this in 7e24c19 May 19, 2012

Owner

caarlos0 commented May 19, 2012

Someone pleaaase test it even more :)

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