Skip to content
This repository has been archived by the owner on Sep 26, 2018. It is now read-only.

Call/SMS reporting:- #65

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shivkumarsah
Copy link
Contributor

@shivkumarsah shivkumarsah commented Jun 29, 2017

Please review Call and SMS report which include the below features:

  • Minute of calls
  • Number of calls
  • Number of SMS

dashboard_reports_calls

@facebook-github-bot
Copy link

@shivkumarsah updated the pull request - view changes

@shivkumarsah shivkumarsah force-pushed the sprint3_pr_call_report branch from 4d36b74 to c06a1e1 Compare August 3, 2017 15:21
@facebook-github-bot
Copy link

@shivkumarsah updated the pull request - view changes

@shaddi
Copy link
Contributor

shaddi commented Aug 4, 2017

Thanks! For changes like this that impact UI, could you please include a screenshot?

Copy link
Contributor

@shaddi shaddi left a 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>
Copy link
Contributor

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>
Copy link
Contributor

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>
Copy link
Contributor

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?

Copy link
Contributor Author

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

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.

3 participants