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

Use new crm-angular-js instead of ng-app to bootstrap AngularJS #20419

Merged
merged 1 commit into from
May 27, 2021

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented May 25, 2021

Overview

Updates the method used to bootstrap Angular for internal consistency.

Before

A few places use the AngularLoader class directly and the old <div ng-app> tag.

After

All core code switched to using the new angularjs.loader service and the new <crm-angular-js> tag.
Using old methods now emit a deprecation warning to remind extension authors to update their code.

Technical Details

The difference between the two is that crm-app (which is provided by AngularJS) can only be used once per page, but crm-angular-js can be reused multiple times per page so that multiple modules inserting apps don't interfere with each other.

Comments

Extensions which inject Angular content include Afform, CiviVolunteer, and CiviCase. Switching them all to use crm-angular-js will prevent collisions.

@civibot
Copy link

civibot bot commented May 25, 2021

(Standard links)

@civibot civibot bot added the master label May 25, 2021
@seamuslee001 seamuslee001 self-assigned this May 25, 2021
@seamuslee001
Copy link
Contributor

@colemanw I tried this on my local and I cleared cache and also disabled the Asset Builder cache but when I tried to create a new mailing (non mosaico) it didn't work it didn't seem to run any angular at all. Dashboard and V4 API seemed to be fine but not Traditional Mailings

traditional-mailing-failing

@colemanw
Copy link
Member Author

@seamuslee001 it wasn't enough to change the tag I had to change the loader server-side too. I went ahead and did further cleanup.

Use new `crm-angular-js` instead of `ng-app` to bootstrap AngularJS
Emit a deprecated warning from the old load() function to alert extension maintainers of the change.
@seamuslee001
Copy link
Contributor

I tested on the PR test site traditional mailings APIv4 and also tried loading up an afform and they all worked. merging

@mattwire
Copy link
Contributor

mattwire commented Dec 1, 2021

@colemanw @seamuslee001 It seems this never got documented and seems pretty fundamental to the way we use angularjs? - https://docs.civicrm.org/dev/en/latest/framework/angular/loader/

I've opened issue https://lab.civicrm.org/documentation/docs/dev/-/issues/872

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

Successfully merging this pull request may close these issues.

3 participants