-
Notifications
You must be signed in to change notification settings - Fork 36
Call/SMS reporting:- #65
base: master
Are you sure you want to change the base?
Call/SMS reporting:- #65
Conversation
cebc8a1
to
4d36b74
Compare
@shivkumarsah updated the pull request - view changes |
4d36b74
to
c06a1e1
Compare
@shivkumarsah updated the pull request - view changes |
Thanks! For changes like this that impact UI, could you please include a screenshot? |
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.
It looks like there's some redundant scripts in here, let's make sure those are organized and not duplicated. Are all of these actually used?
<script src="https://cdnjs.cloudflare.com/ajax/libs/d3/3.1.5/d3.min.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/nvd3/1.1.13-beta/nv.d3.min.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/moment.js/2.9.0/moment.min.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/bootstrap-datetimepicker/4.14.30/js/bootstrap-datetimepicker.min.js"></script> |
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.
This is already included above
<script src="https://cdnjs.cloudflare.com/ajax/libs/nvd3/1.1.13-beta/nv.d3.min.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/moment.js/2.9.0/moment.min.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/bootstrap-datetimepicker/4.14.30/js/bootstrap-datetimepicker.min.js"></script> | ||
<script src="https://cdn.datatables.net/1.10.15/js/jquery.dataTables.min.js"></script> |
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.
This is included above
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/0.12.2/react.min.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/react/0.12.2/JSXTransformer.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/d3/3.1.5/d3.min.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/nvd3/1.1.13-beta/nv.d3.min.js"></script> |
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.
This is included above. Let's keep script imports together?
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.
This is for chat js files. Above includes files are css style. CSS files are included in top of page in 'pagestyle' block to load pages fast, and js files are added in bottom '"js" block
Please review Call and SMS report which include the below features: