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

Fix popup bug when displaying multiple popups #330

Merged
merged 2 commits into from
Oct 6, 2016

Conversation

thevoiceofzeke
Copy link
Contributor

In this PR:

  • Fixed the problem described in MUMUP-2748
  • Minor format changes in services.js to make it a bit easier to read

Copy link
Contributor

@timlevett timlevett left a comment

Choose a reason for hiding this comment

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

Looks good, minor inline suggestion but not required for merge.

var orderedPopups = $filter('orderBy')(unseenPopups, ['popup.startYear', 'popup.startMonth', 'popup.startDay', 'id']);
console.log(orderedPopups);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest wrapping console.log in a if(APP_FLAGS.debug) or use $log.

@@ -80,6 +82,7 @@ define(['angular','require'], function(angular, require) {
clickOutsideToClose: true,
openFrom: 'left',
closeTo: 'right',
preserveScope: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah yes, I remember getting caught by this before.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.04%) to 39.467% when pulling 313e381 on thevoiceofzeke:announcements into 00f2d09 on UW-Madison-DoIT:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 39.537% when pulling 2f1f49b on thevoiceofzeke:announcements into 00f2d09 on UW-Madison-DoIT:master.

@thevoiceofzeke thevoiceofzeke merged commit 01bd6aa into uPortal-Attic:master Oct 6, 2016
@apetro apetro added this to the 2.9.0 milestone Oct 10, 2016
@thevoiceofzeke thevoiceofzeke deleted the announcements branch October 13, 2016 17:39
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.

4 participants