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

fix(datepicker): Fix destroy event for popup #5274

Closed
wants to merge 2 commits into from

Conversation

deeg
Copy link
Contributor

@deeg deeg commented Jan 15, 2016

Closes #5058

Plunker with fix

Click destroy button to see popup go away on first input example.

The div keeps the reference to the element correct because it isn't added and removed to the dom as a clone due to ng-if

@wesleycho
Copy link
Contributor

This LGTM, but since I made the original suggestion, someone else needs to sign off.

@wesleycho wesleycho added this to the 1.1.0 milestone Jan 15, 2016
<ul class="uib-datepicker-popup dropdown-menu" dropdown-nested ng-if="isOpen" ng-style="{top: position.top+'px', left: position.left+'px'}" ng-keydown="keydown($event)" ng-click="$event.stopPropagation()">
<li ng-transclude></li>
<li ng-if="showButtonBar" class="uib-button-bar">
<div>
Copy link
Contributor

Choose a reason for hiding this comment

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

fix indent

@deeg
Copy link
Contributor Author

deeg commented Jan 16, 2016

@icfantv, spaces should be all set. Sorry about that. All lines have changed because indentation was wrong before this fix.

@icfantv
Copy link
Contributor

icfantv commented Jan 16, 2016

Ok, please walk me through how wrapping in a <div> fixes the issue.

@wesleycho wesleycho closed this in 2cb3bc2 Jan 16, 2016
@wesleycho
Copy link
Contributor

Just leaving for posterity, wrapping it in a div is a hack to prevent popup[0] in the popup from being a comment tag due to how ng-if works.

@jonasem
Copy link

jonasem commented Mar 18, 2016

Thanks for clearing that up wesleycho, our code broke when updating deps due to a missing <div> in our version of the template. I would expect the need to rewrite to be listed as a breaking change though (the required <div>-wrapper part) as we had no clue that we had to update our template.

@wesleycho
Copy link
Contributor

Sorry about that, I should have marked this as a breaking change technically.

@icfantv
Copy link
Contributor

icfantv commented Mar 18, 2016

@wesleycho do we need to update the changelog?

@wesleycho
Copy link
Contributor

Technically, we probably should.

@icfantv
Copy link
Contributor

icfantv commented Mar 18, 2016

@jonasem, make sure you use the backtick characters (or escape the angle brackets with a backslash) around any code that could be interpreted by the editor as an actual DOM event. This is why your <div> didn't actually show up. I've edited your comment to escape them.

@jonasem
Copy link

jonasem commented Mar 28, 2016

Thanks :-)

@deeg deeg deleted the fix-datepicker-destroy branch April 12, 2016 13:37
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.

datepicker-popup remains on screen when launched from modal that goes away
4 participants