Skip to content

Dropdown button in dev branch#467

Closed
kasperp wants to merge 4 commits intotwbs:devfrom
kasperp:dropdown-btn-dev
Closed

Dropdown button in dev branch#467
kasperp wants to merge 4 commits intotwbs:devfrom
kasperp:dropdown-btn-dev

Conversation

@kasperp
Copy link
Copy Markdown
Contributor

@kasperp kasperp commented Oct 23, 2011

Moved pull request 192 to dev branch

Comment thread bootstrap.css Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this inline-block for just .dropdown that surrounds the buttons? If so, let's just pull that out and only put it on the .dropdown if possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't do that as I figured that li.menu was for backwards compatibility and eventually would be removed, but no problem I'll pull it out.

@mdo
Copy link
Copy Markdown
Member

mdo commented Oct 23, 2011

Nice, much better merge here :). Taking a look at this in more detail now. I think the HTML and CSS can get merged in no problem, but I'm going to get @fat's opinion on the JS afterwards.

@kasperp
Copy link
Copy Markdown
Contributor Author

kasperp commented Oct 23, 2011

In my notification area (and email) I can se that you made 3 comments, however I can only see one her https://github.com/twitter/bootstrap/pull/467/files, did you change your mind or I'm I looking in the wrong place?

@mdo
Copy link
Copy Markdown
Member

mdo commented Oct 24, 2011

Yeah, I deleted a few of them. They're small enough I can make them.

Only one bug stopping me from working on integrating this now: looks like in Chrome (at least) we have a horizontal scroll bar. Looks like something isn't closing around the topbar/navigation section. Take a look and resolve that please and I can get this in.

@kasperp
Copy link
Copy Markdown
Contributor Author

kasperp commented Oct 24, 2011

An extra </div> made it in, sorry about that. I removed it and changed tabs to spaces.

I also added an additional check for disabled state to the javascript, which didn't make it over in my first move to the dev branch.

@mdo
Copy link
Copy Markdown
Member

mdo commented Oct 26, 2011

@kasperp, just pushed a new temp branch, kasperp-dropdown-btn-dev, that implements this with some adjustments to make it a bit more straightforward to style. Also segments btn dropdowns from regular dropdowns and adds a lot of comments.

Take a look at let me know if anything stands out.

@mdo
Copy link
Copy Markdown
Member

mdo commented Oct 26, 2011

Also, IE7 no likey. Diving in to see if I can make it work...

@kasperp
Copy link
Copy Markdown
Contributor Author

kasperp commented Oct 26, 2011

@markdotto that looks great. I really must improve my comment writing :-). As for the extra class btn-dropdown, I see that make the styles more straightforward. However, I would have preferred not having to add that extra class to the markup, but can live with it no problem.

Yeah - IE7 I haven't tested (sorry I should have mentioned that), great if you can make it work.

@mdo
Copy link
Copy Markdown
Member

mdo commented Nov 1, 2011

Update: still haven't figured out a good way to handle IE7. Will swing back around to this in 2.0-wip.

@kasperp
Copy link
Copy Markdown
Contributor Author

kasperp commented Nov 1, 2011

I'll try and give IE7 a go, when do you plan to release 1.4 ?

@andriijas
Copy link
Copy Markdown
Contributor

"when its ready"?

hopefully soon! its going to rock.

@kasperp
Copy link
Copy Markdown
Contributor Author

kasperp commented Nov 1, 2011

@andriijas fair enough. I was looking for an approximation. If I'm going to give this IE7 issue a go before 1.4 I wanted an idea of the how long I had, 1 day, 1 week, 1 month ....

@andriijas
Copy link
Copy Markdown
Contributor

hey i was just citing what I thought was coming as an answer :) i also wanna know...

@mdo
Copy link
Copy Markdown
Member

mdo commented Nov 1, 2011

We'd like to release by Friday to get this out so we can focus on 2.0-wip.

@kasperp
Copy link
Copy Markdown
Contributor Author

kasperp commented Nov 1, 2011

@markdotto, sounds great I'll try and give it ago before friday

@necolas
Copy link
Copy Markdown
Contributor

necolas commented Nov 1, 2011

Had a quick look at the CSS. This may be unrelated but the IE6/7 inline-block hack usually needs *zoom:1 (or any hasLayout trigger) after this line

@kasperp
Copy link
Copy Markdown
Contributor Author

kasperp commented Nov 3, 2011

I just noticed a difference between the pushed state in my version (dropdown-btn-dev) and your version (kasperp-dropdown-btn-dev).

Mine
Mine

Yours
Yours

Was that intentionally or by accident ?

As for the IE7 issue, I have run out of time this week. So I guess I have to wait for 2.0 for this, one more reason to hope for an early x-mas :-)

@derekprior
Copy link
Copy Markdown

I'm interested in seeing this make it into 1.4. If there's anything I can do to assist with testing or anything like that, please let me know. I have ready access to IE 7, IE 8, and IE 9 (though personally would take this feature with or without IE support).

@kasperp
Copy link
Copy Markdown
Contributor Author

kasperp commented Nov 3, 2011

@derekprior as I just said I'm out of time this week. So if you can provide an IE7 fix that would be awesome.

@kasperp
Copy link
Copy Markdown
Contributor Author

kasperp commented Nov 6, 2011

@markdotto I made a pull request (#573) to your kasperp-dropdown-btn-dev branch which I believe fixes the IE7 problem

@kasperp kasperp closed this Nov 6, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants