Skip to content
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

Rewrite jquery.dashboard.js as Angular app #18971

Merged
merged 1 commit into from
Nov 16, 2020

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Nov 13, 2020

Overview

This is a code rewrite of the home dashboard. The look, feel & functionality is mostly unchanged.
The new dashboard supports embedded afforms (which can contain search kit displays).

Before

Dashboard code based on an old jquery widget which I believe was copied from a Drupal 6 module.

After

Dashboard code based on Angular, APIv4 & flex css.

Technical Details

A dashlet can now specify a "directive" instead of a "url" and that angular directive + its dependencies will be automatically loaded.

@civibot
Copy link

civibot bot commented Nov 13, 2020

(Standard links)

@civibot civibot bot added the master label Nov 13, 2020
@demeritcowboy
Copy link
Contributor

There used to be a refresh button because dashlet output gets cached. I can't see how to refresh them now? I tried removing and re-adding. Even clearing caches didn't seem to update them.

Anyone familiar with the current screen will be able to figure out how to drag dashlets in and out, but I think the old text intro that was at the top was useful for new users and would still be useful here.

If a dashlet itself is angular, do they fight? Nowadays that would pass for entertainment.

@colemanw colemanw force-pushed the angularDashboard branch 2 times, most recently from 68f3e97 to 8973a4f Compare November 14, 2020 21:34
@colemanw
Copy link
Member Author

Thanks @demeritcowboy for taking a look. I've just pushed up a couple changes based on your suggestions:

  1. Added the help text back.
  2. Added a refresh button to the top of each dashlet.

@demeritcowboy
Copy link
Contributor

Cool.

This may or may not be related - I don't see it on dmaster.demo but I do see it on the test site here - creating a new contact on the fly, e.g. from a new contribution, hangs, but it doesn't seem to be a 500 error. I'll see if I can track down if it's related to the PR.

Otherwise I just see some help text that needs updating: These two lines aren't accurate anymore. There's no "configure dashboard" and no "Done" button.

I haven't looked at the code, just r-run, with the stock widgets.

js/Common.js Outdated
// If the server returns an error msg call the error handler
var status = $.isPlainObject(data) && (data.is_error || data.status === 'error') ? 'error' : 'success';
handle(status, data);
})
.fail(function(data) {
},
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah it seems to be these changes. If I apply the common.js diff alone to a site then I get the hang when creating contacts on the fly.

@colemanw colemanw force-pushed the angularDashboard branch 2 times, most recently from f868e6b to 1b591cf Compare November 15, 2020 13:54
@colemanw
Copy link
Member Author

@demeritcowboy ok I'm going to leave changes to Common.js out of this PR and tackle them separately since fixing CRM.status to be compatible with api4 turns out to be trickier than I thought. For now it just flashes "saved" without giving the "saving" message first.
I've also taken out those 2 irrelevant lines from the help file per your suggestion.

@colemanw colemanw force-pushed the angularDashboard branch 2 times, most recently from d64227c to 2951ae1 Compare November 15, 2020 20:47
@colemanw
Copy link
Member Author

Updated a bit more help text, as the Report UI has changed significantly since it was written.

@eileenmcnaughton
Copy link
Contributor

@colemanw please merge this once @demeritcowboy indicates he is happy with it.

The approach / plan has been discussed & agreed and despite the large number of changes I perceive this as a low-risk change because it doesn't have complex interactions on mulitple subsystems & and problems will be very much in dev's faces over the remaining time until release

@eileenmcnaughton
Copy link
Contributor

@colemanw this needs a rebase - blame @seamuslee001

New home dashboard written with Angular + APIv4.
Functionality is mostly unchanged.
The motivation for this is to support afforms embedded within dashboard dashlets.
@colemanw
Copy link
Member Author

Rebased it. Should be good to go. Want to take one last look @demeritcowboy?

@demeritcowboy
Copy link
Contributor

  • General standards
    • [r-explain] PASS
    • [r-user] Non-blocking Comment
      • I understand this PR is about technology, just the main problem with the dashboard has always been that dashlets usually don't fit and get cut off, even if you only fill one column.
    • [r-doc] Issue, but non-blocking
    • [r-run] PASS
      • Works.
      • Permissions still seem to be enforced.
      • Upgrade OK.
      • Did not test shoreditch or other.
      • Did not test custom dashlets.
  • Developer standards
    • [r-tech] PASS
    • [r-code] PASS
      • Only skimmed.
    • [r-maint] ?
    • [r-test] PASS

@colemanw colemanw merged commit 4b263f7 into civicrm:master Nov 16, 2020
@colemanw colemanw deleted the angularDashboard branch November 16, 2020 20:27
@colemanw
Copy link
Member Author

Thanks @demeritcowboy.
I agree we should update the docs too. In fact that whole page is out-of-date.

@adevapp
Copy link

adevapp commented Feb 17, 2021

Hi @colemanw

Some clients are using the dashboard as a live dashboard system, where we want the page to refresh every 15 seconds or so - Otherwise the report appears out of date when we have complex actions.

I have been using a hacky approach to set the dashboard timeout which has worked until now - However it appears to have been deprecated by the changes here and doesn't work in 5.34.0 with the switch to Angular.

function activitytimetracking_civicrm_pageRun(&$page) {
  if (is_a($page, 'CRM_Contact_Page_DashBoard')) {
    $contactDashlets = $page->get_template_vars('contactDashlets');
    if (empty($contactDashlets)) {
      return;
    }
    foreach ($contactDashlets as &$contactDashlet) {
      if (empty($contactDashlet)) {
        continue;
      }
      foreach ($contactDashlet as &$dashlet) {
        if (!empty($dashlet['name'])
          && strpos($dashlet['name'], 'report/') !== FALSE
          && $dashlet['cacheMinutes'] == 1
        ) {
          $dashlet['cacheMinutes'] = 0.25;
        }
      }
    }
    $page->assign('contactDashlets', $contactDashlets);
  }
}

I've looked at the various hooks and it doesn't appear there is a current better approach - Or I can't find a hook I can find use that enables me to alter the data going to the dashboard - Only an angular one where I can modify the HTML content of ~/crmDashboard/Dashlet.html

What approach do you recommend to setting a timeout value on reports that is a float...as part of a hook? The database cache time out field only allows for integers, and 0 causes a constant refresh of the page. Is the solution for a change request to make this field a float?

FYI - I did test some code which triggers the refresh function on your Angular refresh code - This seems hacky and not ideal for long term maintenance.

      let reportMainClass = '';
      if ('{/literal}{$reportContext}{literal}' == 'dashlet') {
        reportMainClass = ' .widget-report-{/literal}{$instanceId}{literal} ';
      }

      function forceRefresh() {
        $(reportMainClass).closest('.crm-dashlet').find('a.fa-refresh').trigger('click');
      }

@colemanw
Copy link
Member Author

It seems like the best approach would be to allow non-integers & make that field a float as you suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants