-
Notifications
You must be signed in to change notification settings - Fork 49
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
Winter edition #449
Winter edition #449
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One cosmetic change before merging to master
.
Could you please also update the CHANGELOG
and add an instruction in the description of this PR about what is this feature, add the screenshot and instruction on how to enable it.
@@ -43,8 +43,10 @@ define(['angularAMD'], function (angularAMD) { | |||
var isChristmas = isDateWithinMargin(TODAY, CHRISTMAS_DATE, DAY_MARGIN); | |||
|
|||
if (isChristmas) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we please add additional flag, e.g. winterEditionEnabled
and make the condition:
if (winterEditionEnabled && isChristmas) {
?
It should be set to false
by default.
If admin of an AET instance decides to enable it, it should be manually set to true
on the server instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -42,9 +45,11 @@ define(['angularAMD'], function (angularAMD) { | |||
var DAY_MARGIN = 14; | |||
var isChristmas = isDateWithinMargin(TODAY, CHRISTMAS_DATE, DAY_MARGIN); | |||
|
|||
if (isChristmas) { | |||
if (isChristmas && winterEditionEnabled) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert the order of flags.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Added description and screenshot to PR. |
Updated winter edition with improvements suggested by users.
Description
Now the Winter Theme is disabled by default. To enable it go to
report/src/main/webapp/app/themes/winterEdition.directive.js
and change the value ofwinterEditionEnabled
variable totrue
. Winter Theme will be visible 14 days before and after Christmas. Winter Theme contains:Screenshot
Types of changes
Checklist:
I hereby agree to the terms of the AET Contributor License Agreement.