Not correctly calculating taggedElement margin.

[danballance]

Hi,

We are loving this project and using it to load a shopping cart on our marketing site. So thanks in advance for all of the great work.

One issue we are hitting is where a modal dialogue opens and the height calculation returns the correct bottom position of the modal, but "crops" our drop shadow breaking the illusion that the cart is actually in the page.

Would you be open to me submitting a pull request for a couple of extra options, say: heightModifier and widthModifier ?

So my idea is that after the height or width calculation has been done, this modifier would be applied. We would like to use this to add 20 or 30 pixels to the height calculation so that we don't lose our drop shadow.

Would you be interested in this as a new feature? Do you think I am approaching the problem in the correct way?

many thanks in advance,

Dan

[davidjbradshaw]

The issue your having can be addressed by changing the heightCalaculationMethod. Your issue is that the modal dialogue is extending out side of the body tag. I would suggest looking at taggedElement and tagging the dialogue and the body tag.

[davidjbradshaw]

Oh just read that a bit more, can you not put an outer div around your model?

[danballance]

Hi,

Thanks for coming back to me :) We've been using lowestElement and taggedElement with some success - but it clips some drop shadow. One option could be too try and wrap the modal somehow, but this will be fairly awkward to do because we are using AngularUI ( https://angular-ui.github.io/bootstrap ) to hook into Bootstrap's modals. So those components generate that extra markup and we have little control.

What you say is a reasonable suggestion though. I'll go and take another look and report back here shortly. If I can do it with a wrapper div I'll close the ticket. Cheers!

[davidjbradshaw]

The other maybe easier option is to add a bottom margin to your model.

[danballance]

Hello again.

Unfortunately the margin-bottom technique does not seem to work for me. The height calculation seems to always ignore any margin that I add.

And in a similar way, the wrapper div doesn't work either. I can add padding (which doesn't help me) but any margin is ignored.

I haven't looked closely at your code yet, but it seems as if the height calculation doesn't take into account anything out side of the box - so margin and border seem to get lost. Is that true do you think? It could be something to do with the way the modal is implemented, but might we actually need the calculations to be aware of the margin and border too?

[davidjbradshaw]

The margin thing for lowestElement and taggedElement was fixed in v3.2.0.

[danballance]

Okay, weird, we're using v3.2.0 - 2015-09-04. I've just upgraded to 3.5 to see if this helps, but not joy.

Trying to dig into this a bit, I can see that getComputedBodyStyle('margin'+Side) is returning 0 for me. I'm trying to understand what this function does. It seems to get the marginBottom from the body element? Is that right?

[danballance]

In fact, having done some testing, if I set this in my CSS it works:

body {
margin-bottom: 30px;
}

This seems an acceptable workaround for me - I can set some margin-bottom on my body element without any problems. But should this be considered a bug? I was expecting the margin bottom to be detected from the tagged element.

[davidjbradshaw]

ah yes that is the bug, getting wrong margin!

[davidjbradshaw]

Welcome a PR if you have a moment to fix it.

[danballance]

Okay, I'll try and make a fix today.

[danballance]

Hi, I thought I'd send a quick update on this. I took a quick look at fixing this issue - and I think it looks quite straight forward to fix. However, it actually seems to have made my particular use-case worse. My scenario is this:

• using lowestElement my modal is not included in the calculation
• using taggedElement the inner divs that I control in my modal are included in the calculation, but detecting their margin does not help me. It's an outer bootstrap div that I need to add the margin to and I cannot tag it.
• so with the code as it currently is, adding margin-bottom to my body element actually allows me to add an arbitrary number of extra pixels to the taggedElement calculation.

I hope that makes sense. I would like to fix this issue for you - it doesn't make sense to me to look at the body element for the margin calculation. However, if I fix this issue, I will be back to my original situation of needing to add two new settings: widthModification and heightModification. My proposal is that if these are set, the margin is not detected and these values are used instead.

This is quite a lot more involved than the bug fix. I've started to take a look at this but it's not finished yet. Before I waste too much time, is this a pull request that you might consider?

Or should we just live with the behaviour as it is? Setting the margin on the body does work for me - although it is a strange technique.

[danballance]

Just to confirm, I have a provisional version of this working now on my local machine that checks the margins on the element being processed and also accepts the additional settings of widthModification and heightModification. It's not unit tested or linted yet though. If you think it's worth a pull request I'll get it into a proper state to send across to you.

[davidjbradshaw]

That's strange if an element can be tagged, lowestElement must be able to see it. As they both use the same function to process elements.

I'd like to see your bug fix, I'm pretty sure that once that is got right, lowestElement will work for you and there won't be a need to modification settings.. Is it on a git branch I can look at?

[danballance]

Hi,

Thanks for your help with this. You may well be right, I'll check my code again and push a branch for you to look at.

The issue I am having is very slightly different to what you are describing though. The modal div that I am tagging is an inner div. If I add margin to this, it just pushes the drop shadow further away. So this tagged element + margin does not help me.

For some reason, getting the lowest element does not pick up the bootstrap generated divs with the drop shadow. So lowest doesn't seem to work for me, although I will check this again in more detail to be sure.

So I'm still left with this scenario where I really need to do taggedElement + arbitrary additional number of pixels. So the taggedElement is the lowest element I can control - but not the lowest element. There are some generated wrapper divs with a fixed number of pixels drop shadow. So one way to deal with this is to use taggedElement + modifier.

Another angle to take now would be for me to try and understand more about why those bootstrap generated divs do not seem to be part of the lowest calculation. I'll check the code out a bit further to make sure I haven't made a mistake and then update again here.

[davidjbradshaw]

If you make a simple test case with the bootstrap model I'll have a quick look. You can add margin to the bootstrap div via CSS.

[danballance]

Hi,

I stand corrected on the need for extra parameters. It seems that the bug fix in the above pull request with lowestElement does solve the problem.