Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

feat(bootstrap3): Add support for most Bootstrap 3 changes. #742

Closed
wants to merge 41 commits into from

Conversation

hallister
Copy link

Probably best to close the other two open issues and stick this in 1 PR. I'll close #729 and #731 to consolidate all of this.

@avillalain
Copy link

@hall5714 I was testing the changes on the plunker I made with the same fix for the collapse PR I made before, and yep its broken (YEAH!!!). I was looking at their changes for collapse and their logic is not different. But I think the example in the previous plunker I posted here is incorrect with the name of the navbar classes, but if you try the dropdown links they work just fine.

As a side note they have broken also their navbar. If you want to see it for yourself just reduce the viewport until the toggable button appears and click on it to expand the navbar, and voila! a scrollbar appears!!!

@hallister
Copy link
Author

@jonjaques Would you mind PRing @24c5aed and @ada446f as a separate PR against the new bootstrap3 branch on here? Trying to separate out these PR's to make this simple on everyone.

@hallister
Copy link
Author

@pkozlowski-opensource That should do it for the immediate fixes. Dialog and Modal are waiting on your PR, Accordion and Collapse need a major refactor, Progressbar needs a PR completed before we can make styling changes and Tooltip needs the arrow positioning issue solved for focus triggers. Since Progressbar and Dialog/Modal are the most immediate (since they have PRs ready) I'll do those once the PR's are complete and then we can worry about Accordion/Collapse and Tooltip.

@pkozlowski-opensource
Copy link
Member

@hall5714 you did grand job on this one, pretty amazing! I've merged now all the pull requests and the bootstrap3 branch starts to take some shape. I believe that we still need:

  • handle missing directives:
    • accordion / collapse
    • modal
    • progress bar
  • review demo page for Bootstrap 3
  • update to the final release version

I believe that this issue can be closed now, let's open separate issues / pull request from now on.

@hall5714 once again, your help is invaluable. It would take us far more time to get to Bootstrap 3 support without your help! True open source spirit!!!

@bcronje
Copy link

bcronje commented Aug 20, 2013

@hall5714 once again, your help is invaluable. It would take us far more time to get to Bootstrap 3 support without your help! True open source spirit!!!

+1 to this!

@hallister
Copy link
Author

Glad I could help :).

@mfrye
Copy link

mfrye commented Aug 22, 2013

+1 to @hall5714. Yeah man you've done a ton of work on this.

@joshkurz
Copy link
Contributor

+1

On Thu, Aug 22, 2013 at 9:42 AM, Michael Frye notifications@github.comwrote:

+1 to @hall5714 https://github.com/hall5714. Yeah man you've done a ton
of work on this.


Reply to this email directly or view it on GitHubhttps://github.com//pull/742#issuecomment-23090539
.

Josh Kurz
www.atlantacarlocksmith.com http://www.atlantacarlocksmith.com

@diorray
Copy link

diorray commented Aug 23, 2013

@hall5714 you did a really great work, thank you

@ojraqueno
Copy link

+1

4 similar comments
@vpegado
Copy link

vpegado commented Sep 23, 2013

+1

@gabzim
Copy link

gabzim commented Oct 9, 2013

+1

@kristianmandrup
Copy link

+1

@j8
Copy link

j8 commented Oct 28, 2013

+1

@elirov
Copy link

elirov commented Nov 6, 2013

any progress on this?

@kristianmandrup
Copy link

Look in the forks overview. Starting from Sept 7th you will see the bootstrap3_bis2 branch forging ahead, then stalling.
They obviously need more community help/support to complete this effort. So instead of just giving "thumbs up" +1, give them a real hand with some code patches, tests etc. and we will all benefit - the sooner the better ;)

@pkozlowski-opensource
Copy link
Member

Thnx for this comment @kristianmandrup ! You are right, this is volunteer-run project in the spare time of commiters. Putting more pressure on us want make things happen faster. What can make a difference is help with testing, bug tracking and fixes. Thank you.

@kristianmandrup
Copy link

I think the first thing to be done is to get a handle of the current
bootstrap 3 branches. Start with bis_2, run the tests see what is working,
what breaks, what looks to be missing.
The look at some of the other (smaller) patch branches such as modal and
progress (I think). Then try merging them in, one by one - while
(manually?) merging, running tests, see how it looks now.
Create a Bootstrap3-Status.md document which outlines the current status
of features, functionalities etc that are working and which still need some
love (for others to more easily join the effort).

Then slowly progress from here... also make a document
Bootstrap3-Missing-Features.md which outlines what features are still
missing and perhaps even a Bootstrap3-Issues.md document, listing
remaining issues to be resolved (perhaps even with links to specific
tests). Depending on the status, missing features and issues, the could be
wrapped into 1, 2 or 3 document like this.

I think this is the best way forward. Then call the new branch simply
"bootstrap3-dev" and make another one "bootstrap3-stable" containing only
working features and where all tests pass.
Just my take on this. what do you think?

I might have a go at this tonight or this weekend sometime... but please
feel free to get going ;) Let's have some input and ideas on how to best
proceed.

On Fri, Nov 8, 2013 at 4:26 PM, Leon Radley notifications@github.comwrote:

I'd be happy to help, but It's a bit overwhelming. I wouldn't know where
to start.
How can we better work together?
Could we create a beta1 milestone for the bootstrap 3 conversion and put
some issues into that milestone that needs fixing.

At the moment there are a lot of commits and they are for both BS2 and BS3.

Then anyone that feels they could contribute could "claim" a issue by
simply adding a comment to the issue, and when they are done, submit a pull
request.

I'm willing to do the organisation if could get some access :)


Reply to this email directly or view it on GitHubhttps://github.com//pull/742#issuecomment-28070527
.

@kristianmandrup
Copy link

Raining like crazy here in Strasbourg. What else to do than work on this Bootstrap 3 support all night until I crash ;)
About time we get another spurt of progress on this... and I get a deep dive into Bootstrap 3 and Angular directives as well, what's not to like :)

@kristianmandrup
Copy link

Just been trying to take a twak at this! Here is what I learned:

@Zmetser looks like a good starting point. All test pass :)
https://github.com/Zmetser/bootstrap/tree/bootstrap3_bis2

@Corydon has some IE8 and memory leak fixes which would be nice to include
https://github.com/Coridyn/bootstrap/compare/cfh_bis2_progressPatch_ie8

Linting src/tabs/tabs.js...ERROR
[L262:C11] W033: Missing semicolon.
        })

Chrome 31.0 (Mac) collapse directive dynamic content should shrink accordingly when content size inside collapse decreases on subsequent use FAILED
    Expected 18 to be less than 18.
collapse.spec.js:103:32)
Chrome 31.0 (Mac) tooltip with specified popup delay should not open if mouseleave before timeout FAILED
    Error: No deferred tasks to be flushed

@bsundrod is adding angular1.2-rc.3 support
https://github.com/bsundsrud/bootstrap/tree/bootstrap3_angular12

@benaghaeipour has tried to improve the collapse functionality, using a deferred query and other tweaks
branch: bootstrap3_bis2

benaghaeipour@51c1e27

He also improved accordion-group and accordion spec
benaghaeipour@b963b03

Chrome 31.0 (Mac): Executed 482 of 482 SUCCESS (24.783 secs / 24.13 secs)

@trask also made some patches at https://github.com/trask/angular-ui-bootstrap/tree/bootstrap3_bis2+patch , notably for the typeahead controller

Chrome 31.0 (Mac): Executed 497 of 497 SUCCESS

So @benaghaeipour and @Zmetser look like good candidates for a baseline. These two forks could most likely be merged without too much fuzz. Then progressively merge @Corydon and @trask fixes/patches. That should get us closer!

Finally make a angular12-dev branch and disable the timepicker.spec for now to focus on other errors.

Would also be nice to upgrade jQuery to at least 1.10 or 2.0.3 support for the angular12-dev branch IMO

@kristianmandrup
Copy link

Been trying to merge @Zmetser with @benaghaeipour @trask and @Corydon this morning.

Resulting branch can be found here: https://github.com/kristianmandrup/bootstrap/tree/bootstrap3-dev

Only one minor issue (tooltip spec). I commented out $timeout.flush(); since it throws error coz there is nothing to flush. I think this could be due to memory leak fixes by @Corydon ? I asked him on him commit.

    it('should not open if mouseleave before timeout', inject(function ($timeout) {
      elm.trigger('mouseenter');
      expect(elmScope.tt_isOpen).toBe(false);

      elm.trigger('mouseleave');
      // $timeout.flush();
      expect(elmScope.tt_isOpen).toBe(false);
    }));    

But please feel free to work from here and perhaps branch off with an Angular12 branch as well, perhaps by merging in @bsondrud:bootstrap3_angular12

Cheers!

PS: I think that as soon as we get support for Angular 1.1 and 1.2, this branch is ready for prime time.

@kristianmandrup
Copy link

I just created the following branch boostrap3-angular12-dev as a baseline for work on Angular 1.2.0 support.

Please see DEV_STATUS.md for the current status and how you can help make progress on this. A little help would go a looong way!

@pkozlowski-opensource
Copy link
Member

@kristianmandrup thnx so much for looking into this and all the effort. Sorry guys that I wasn't available to drive those efforts, should have a bit more time from now on.

So, looking at your comments I'm a bit worried that we are trying to tackle both AngularJS1.2 and Bootstrap3, I think we should take one chunk at a time. The current master branch has many fixes fro AngularJS1.2 and the bs3 branch needs to be rebased on the current master.

I know that there are many branches and it is not clear which one should be worked on so I'm going to clean it up now (as far as I can...) and rebase bs3-specific branch on top of the current master (that is still bs2 specific but has good compatibility with 1.2).

If I might suggest the approach here, let's tackle ng1.2 and bs3 separately. Let's make current master (code and demo) work with ng1.2 and then focus on BS3 fixes.

@kristianmandrup what do you think? Could you try to run tests of the current master with ng1.2 and check the demo page opening PRs / issues for each encountered item (a separate issue for each item)? This way we are going to have a set of issues to labels with BS3 / Ng1.2, each issue should be small and people could jump in to help with fixes.

I would also love to move the discussion out of this issue as it is closed and we are starting to get messy comments in there.

@kristianmandrup
Copy link

Hi Pawel, I agree with your statements and strategy. I will see what I can do this weekend.
We need some other forum to have a better discussion of how to proceed and make sufficient progress on this.
At least, the current bs3 branch looks pretty usable for Angular 1.0x already I think.

@pkozlowski-opensource
Copy link
Member

@kristianmandrup we can use angular-ui mailing list (angular-ui@googlegroups.com) for more general discussions.

To coordinate efforts on particular issues I've created labels for both BS3 anf NG1.2 and opened some BS3-specific issues. Hopefully folks that want to contribute are going to be able to pick individual issues from the BS3 label:
https://github.com/angular-ui/bootstrap/issues?labels=Bootstrap+3.0&milestone=&page=1&state=open

@danielsnider
Copy link

+1

@pkozlowski-opensource
Copy link
Member

@danielsnider commenting on the closed issue is not going to get us to the BS3 support any faster. We are 4 issues away from releasing BS3 support, all the practical help would be appreciated:
https://github.com/angular-ui/bootstrap/issues?milestone=6&state=open

@danielsnider
Copy link

Ok, thanks for the note. I might try to fix the Time-picker issue.

@hackdan
Copy link

hackdan commented Dec 10, 2013

+1

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

Successfully merging this pull request may close these issues.

Bootstrap 3.0 list of issues