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

[REF] Decouple crmD3 angular module from CiviMail #19047

Merged
merged 1 commit into from
Nov 26, 2020

Conversation

colemanw
Copy link
Member

Overview

Makes the D3 graphing library load in a more standard way.

Before

The crmD3 module was being conditionally declared based on whether CiviMail is enabled, then automatically loaded on the civicrm/a base page.

After

crmD3 module always exists, but is not automatically loaded on any base page. The crmMailingAB will load it as a dependency.

Technical Details

From the user's POV there is no change here. It just manages the Angular dependencies in a more standard, reusable way.

Comments

This is with an eye toward using D3 for Search Kit displays.

The crmD3 module was being conditionally loaded based on whether CiviMail was enabled.
This changes it to always exist but only loaded as required.
@civibot
Copy link

civibot bot commented Nov 25, 2020

(Standard links)

@civibot civibot bot added the master label Nov 25, 2020
@colemanw colemanw changed the title Decouple crmD3 angular module from CiviMail [REF] Decouple crmD3 angular module from CiviMail Nov 25, 2020
@seamuslee001
Copy link
Contributor

Jenkins re test this please

@totten
Copy link
Member

totten commented Nov 25, 2020

This (REF) is a good idea. Now that we have conditional loading (basePages/requires), this looks much more sensible.

@seamuslee001
Copy link
Contributor

I tested this saw there was no errors in the console log and also i see that the relevant code in the stats section

is commented out so I think this is fine to merge

@seamuslee001 seamuslee001 merged commit 6960dd4 into civicrm:master Nov 26, 2020
@seamuslee001 seamuslee001 deleted the crmD3 branch November 26, 2020 00:30
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.

3 participants