Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added getWidth function to correctly calculate parent's width and account for padding in parent. #21

Closed
wants to merge 7 commits into from

Conversation

andycarlberg
Copy link

Base fork version failed to account for padding. offsetWidth doesn't account for padding on it's own. There was also a magic number '6' (perhaps to account for padding?). Correctly calculated available width of parent now.

Previous version failed to account for padding.  offsetWidth doesn't account for padding on it's own.  There was also a magic number '6' (perhaps to account for padding?).  Correctly calculated available width of parent now.
@andycarlberg
Copy link
Author

Sorry, this is phrased poorly. Using offsetWidth tries to make the target element fit the ratio of the parent including it's padding. We want it to fit within the width of the parent without padding. The new function calculates the appropriate width value without the padding. It is also possible the function should account for border, however, I did not test this.

If the element in question does not currently have a width (possibly due
to not being shown at the time), ratio is NaN.  This will correct a NaN
ratio to 0.
Changed timeout to interval so the resizer is constantly checking.  This will let the resizer update if the innerHTML changes.  Could not find an event to tie to so this is the best fix I could come up with.
@patrickmarabeas
Copy link
Owner

Tanks, I'll have to give this a look through.

Unsure why $interval is required - scope.$watch on the next line should achieve exactly that.

@andycarlberg
Copy link
Author

Ah, you're right. I don't know what I was thinking, must have been asleep at the wheel. I changed it back and used mg-model. It works correctly for me now.

had the wrong style name in the getWidth function.  That's what I get for using an emulator...
Still had some extra code from switching timeout to interval that was causing problems.  Really should have read the documentation better...
@SuricateCan
Copy link

Is this PR ever going to be merged?

@helarqjsc
Copy link

Is the project completely abandoned at this point?

@patrickmarabeas
Copy link
Owner

No, life just getting in the way.

Would be helpful if bugs had demos so fixes can actually be tested.

@patrickmarabeas
Copy link
Owner

Please confirm whether v4.2.2 works/doesn't work; code base has changed a fair amount since this PR - simply applying the getWidth function doesn't solve the magic number 6 in context of demo.

Please create a codepen demonstrating issue and update branch with master.

@patrickmarabeas
Copy link
Owner

patrickmarabeas commented Mar 3, 2017

Additionally if you can confirm #53 fixes the issue, I'll merge that in as it doesn't look to break existing demos (v4.2.3)

@andycarlberg
Copy link
Author

I know this is years too late but I'm reviewing some old repos and found this PR again. I believe #53 fixes the issue and is obviously a more up to date PR. I'm happy closing this one.

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

Successfully merging this pull request may close these issues.

5 participants