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

NFC: Improved documentation of crmPageTitle directive. #13337

Merged
merged 1 commit into from
Dec 21, 2018

Conversation

universalhandle
Copy link
Contributor

Overview

Documented assumption that AngularJS base page be named "CiviCRM".

Comments

Is there interest in removing this requirement? It seems like the values for pageTitle and documentTitle could be derived in the link function and assigned to local variables instead of being hardcoded to global variables. My gut, however, tells me that perhaps the code was written this way for a reason (i.e., mysterious bugs).

It is a little unusual that update is wrapped in a $timeout. If I had to guess, it's to keep the $watch function from triggering unnecessarily, since update changes both parts of the watched expression. Does this effectively make the change atomic, rather than having the $watch function fire once when pageTitle is updated and again when documentTitle is updated?

Documented assumption that AngularJS base page be named "CiviCRM."
@civibot
Copy link

civibot bot commented Dec 20, 2018

(Standard links)

@civibot civibot bot added the master label Dec 20, 2018
@colemanw colemanw merged commit 39c3fbc into civicrm:master Dec 21, 2018
@colemanw
Copy link
Member

Basically the timeout is used throughout our angular code to prevent race conditions.

universalhandle added a commit to ginkgostreet/org.chorusamerica.orgdash that referenced this pull request Jan 4, 2019
organization contact information available to all states.

- Added error page for invalid contact IDs.
- Updated page title to include the name of the organization contact
  (note: for the crmPageTitle directive to work, the AngularJS base
  page cannot have a custom title -- it must be named "CiviCRM";
  see civicrm/civicrm-core#13337).
- Updated file name for DashCtrl to match conventions.
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.

2 participants