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

Create widget type to accommodate time-sensitive content #524

Merged
merged 11 commits into from
Sep 21, 2017

Conversation

thevoiceofzeke
Copy link
Contributor

@thevoiceofzeke thevoiceofzeke commented Sep 14, 2017

MUMUP-2780: "Apply lessons learned to refactor Annual Benefits Enrollment widget."

In this PR

  • Establish new widget type for displaying time-sensitive content during one or more provided ranges of days/times, and fall back on "basic" presentation outside of those ranges

Screenshots

screen shot 2017-09-14 at 1 55 00 pm
screen shot 2017-09-14 at 1 57 12 pm
screen shot 2017-09-14 at 2 07 21 pm
screen shot 2017-09-14 at 2 16 02 pm


Contributor License Agreement adherence:

/**
*
*/
var configureAnnualBenefitsEnrollmentContent = function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

To generalize this so it doesn't need annual-benefits-enrollment-specific stuff: have it compute the days until the next template switch and expose that integer to the template.

As in, it's currently showing the before-benefit-enrollment-starts content. And it knows the date on which it'll switch to the next template. So it computes the quantity days until that happens, and exposes it to the current template as daysRemainingShowingThisTemplate or daysUntilNextTemplate or whatever. And then in the actual ABE content, the before-enrollment template uses, references this variable to get at the count.

So then the ABE start date arrives. Boing, onto next template. And that template gets the count of days until the template after it. So the during-ABE content could include the countdown until the end of ABE.

Turtles all the way down.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One thing that doesn't account for is the warning that the active period is going to begin. ABE widget has the countdown during the active period, but also has a "Begins [given date]" case leading up to it.

Copy link
Contributor

@ChristianMurphy ChristianMurphy 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 👍

I'd highly recommend importing Moment for any and all date processing.
I've left some notes on what Moment functions could help. 📓 🕐

$scope.enrollmentPeriodStatus = 'ongoing';
}
// Determine whether enrollment period is upcoming or already over
} else if ($filter('filterDifferenceFromDate')(
Copy link
Contributor

Choose a reason for hiding this comment

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

if (value.isAnnual) {
startDate = (new Date).getFullYear() + '-' + startDate;
endDate = (new Date).getFullYear() + '-' + endDate;
if ($filter('filterDateRange')(startDate, endDate)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

*/
var configureAnnualBenefitsEnrollmentContent = function() {
// Set enrollment period dates for current year
$scope.enrollStartDate = (new Date).getFullYear() + '-'
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would LOVE to use moment. Hopefully this gets the conversation started again :)

if ($filter('filterDateRange')(
$scope.enrollStartDate, $scope.enrollEndDate)) {
// Calculate countdown
$scope.daysLeft = $filter('filterDifferenceFromDate')(
Copy link
Contributor

Choose a reason for hiding this comment

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


define(['angular'], function(angular) {
return angular.module('portal.widgets.filters', [])
.filter('filterDateRange', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

return today >= rangeStart && today < rangeEnd;
};
})
.filter('filterDifferenceFromDate', function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

} else if ($filter('filterDifferenceFromDate')(
$scope.enrollStartDate) > 0) {
$scope.enrollmentPeriodStatus = 'upcoming';
} else if ($filter('filterDifferenceFromDate')(
Copy link
Contributor

Choose a reason for hiding this comment

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

// If date range is annual, not for a specific year,
// use current year in range
if (value.isAnnual) {
startDate = (new Date).getFullYear() + '-' + startDate;
Copy link
Contributor

Choose a reason for hiding this comment

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

// use current year in range
if (value.isAnnual) {
startDate = (new Date).getFullYear() + '-' + startDate;
endDate = (new Date).getFullYear() + '-' + endDate;
Copy link
Contributor

Choose a reason for hiding this comment

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

@ChristianMurphy
Copy link
Contributor

📓 👷‍♂️ 🏁 AppVeyor CI error appears to be the same one from #523
⛔ 👷‍♂️ 🐧 Files appear to be missing license header

[ERROR] Failed to execute goal com.mycila:license-maven-plugin:3.0:check (default-cli) on project uw-frame: Some files do not have the expected license header -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoExecutionException

https://travis-ci.org/UW-Madison-DoIT/uw-frame/jobs/275607992#L725-L731

@thevoiceofzeke thevoiceofzeke changed the title [WIP] Create widget type to accommodate annual benefits enrollment [WIP] Create widget type to accommodate time-sensitive content Sep 14, 2017
@vertein
Copy link
Contributor

vertein commented Sep 15, 2017

thevoiceofzeke#2

})
.filter('filterForDateWithYear', function() {
return function(date) {
var validWithYear = /([12]\d{3}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01]))/;
Copy link
Contributor

Choose a reason for hiding this comment

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

🐎 to limit performance implications consider anchoring the start and the end

/^([12]\d{3}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01]))$/

📓 https://momentjs.com/docs/#/parsing/is-valid/ is a wonderful thing

.filter('filterForDateWithYear', function() {
return function(date) {
var validWithYear = /([12]\d{3}-(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01]))/;
var validDayAndMonth = /(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])/;
Copy link
Contributor

Choose a reason for hiding this comment

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

🐎 to limit performance implications consider anchoring the start and the end

/^(0[1-9]|1[0-2])-(0[1-9]|[12]\d|3[01])$/

📓 https://momentjs.com/docs/#/parsing/is-valid/ is a wonderful thing

@thevoiceofzeke thevoiceofzeke changed the title [WIP] Create widget type to accommodate time-sensitive content Create widget type to accommodate time-sensitive content Sep 18, 2017
Copy link
Contributor

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

👷‍♂️ ⛔

docs\make-a-widget.md
            390:5  warning  Incorrect indentation before bullet: remove 2 spaces  list-item-bullet-indent     remark-lint
            391:5  warning  Incorrect indentation before bullet: remove 2 spaces  list-item-bullet-indent     remark-lint
            392:5  warning  Incorrect indentation before bullet: remove 2 spaces  list-item-bullet-indent     remark-lint
            393:5  warning  Incorrect indentation before bullet: remove 2 spaces  list-item-bullet-indent     remark-lint
  395:133-395:136  warning  Found reference to undefined definition               no-undefined-references     remark-lint
  395:133-395:136  warning  Use the trailing [] on reference links                no-shortcut-reference-link  remark-lint
  395:304-395:307  warning  Found reference to undefined definition               no-undefined-references     remark-lint
  395:304-395:307  warning  Use the trailing [] on reference links                no-shortcut-reference-link  remark-lint
            398:5  warning  Incorrect indentation before bullet: remove 2 spaces  list-item-bullet-indent     remark-lint
            399:5  warning  Incorrect indentation before bullet: remove 2 spaces  list-item-bullet-indent     remark-lint
  414:419-414:440  warning  Use the trailing [] on reference links                no-shortcut-reference-link  remark-lint
  414:419-414:440  warning  Found reference to undefined definition               no-undefined-references     remark-lint
  415:412-415:431  warning  Use the trailing [] on reference links                no-shortcut-reference-link  remark-lint
  415:412-415:431  warning  Found reference to undefined definition               no-undefined-references     remark-lint

https://ci.appveyor.com/project/vertein/uw-frame/build/1.0.388#L1111

Copy link
Contributor

@vertein vertein left a comment

Choose a reason for hiding this comment

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

Looks like the lint issues have been resolved.

Copy link
Contributor

@ChristianMurphy ChristianMurphy left a comment

Choose a reason for hiding this comment

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

most excellent

@thevoiceofzeke thevoiceofzeke merged commit 3b64726 into uPortal-Attic:master Sep 21, 2017
@davidmsibley davidmsibley added this to the 5.2.1 milestone Sep 21, 2017
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.

None yet

5 participants