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

Conversation

@maxisam
Copy link

@maxisam maxisam commented Dec 17, 2014

To work with bootstrap 3.3.1. Ref #2970

To work with bootstrap 3.3.1. Ref angular-ui#2970
@maxisam maxisam changed the title Update modal.js Update modal.js to handle backdrop issue in bootstrap 3.3.1 Dec 17, 2014
@maxisam
Copy link
Author

maxisam commented Dec 17, 2014

Here is the demo. I think this is a better solution than the one I proposed before in #2984. Feedback is welcome !

@maxisam maxisam closed this Dec 17, 2014
@maxisam maxisam reopened this Dec 17, 2014
@pursual
Copy link

pursual commented Dec 23, 2014

Waiting on a fix like this.

@jmblog
Copy link

jmblog commented Dec 26, 2014

+1

@prathiraj
Copy link

+1, waiting for this fix

@allaire
Copy link

allaire commented Dec 31, 2014

Temp. fix:

html {
  height: 100%;
}

body {
  min-height: 100%;
}

.modal-backdrop {
  bottom: 0;
}

@twwwt
Copy link

twwwt commented Jan 1, 2015

+1

1 similar comment
@skmedia
Copy link

skmedia commented Jan 7, 2015

+1

@tony0918
Copy link

tony0918 commented Jan 8, 2015

Any ETA? Thanks.

@karianna
Copy link
Contributor

karianna commented Jan 8, 2015

@maxisam Can you add a comment above the $(angular.element...) line stating the reason for it. I think this PR is sane, but it'll allow more experienced folks than myself to see why it was added and enhance/revert as needed.

@pkozlowski-opensource
Copy link
Member

I'm sorry but we can't merge this one as it assumes certain HTML structure of the backdrop template. We can't do this as people might want (and do) override default templates.

@maxisam
Copy link
Author

maxisam commented Jan 8, 2015

@pkozlowski-opensource I thought about that too. But this project is for Bootstrap 3. It makes sense that we need to use modal-backdrop class in the template because it is in the original Bootstrap 3 and it is actually in the code not in the template.

var angularBackgroundDomEl = angular.element('< div modal-backdrop >< /div >');

I do believe that we have different ways to resolve the issue, but I think it would require to change more than one line and one place.

btw, I think all attempt fixes for this that achieved through css are not proper, since that will cause the issue that original BS team discovered, which was the reason they made this change.

@pkozlowski-opensource
Copy link
Member

@maxisam we don't need to have one-liner fix. If a more generic solution requires more code so let it be. How about writing a dedicated directive that sets a height of a given element to scrollHeight and invoking this directive from a template?

@pkozlowski-opensource
Copy link
Member

We shouldn't be afraid of creating small, focused directives - ones that are doing one thing well, are easy to test etc. The advantage of this approach is that people then can compose those smaller primitives.

@maxisam
Copy link
Author

maxisam commented Jan 8, 2015

@pkozlowski-opensource do you mean we create a directive invoked by the backdrop to set its height with the scrollHeight of modalwindow ?

@kadamska
Copy link

@allaire your fix doesn't work for me when the modal is invoked after scrolling down... It works fine with no scroll offset. UPDATE: Adding "position: fixed" to .modal-backdrop fixes this - quite literally!

@harsh55
Copy link

harsh55 commented Feb 3, 2015

@kadamska : Just add this
.modal-backdrop{
bottom: 0;
position: fixed;
}
It will work for sure. 👍

@xtreemrage
Copy link

@harsh55: it works! 👍

@nunesbeto
Copy link

Using this approach also @harsh55 👍

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.