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

dev/core#4305 - Fix pluses being turned into spaces in hook_alterAngular #26286

Merged
merged 1 commit into from
May 21, 2023

Conversation

demeritcowboy
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/core/-/issues/4305

Before

  1. Implement hook_alterAngular on a file that contains an href tag where its value is an angular expression using +.
  2. Even if your hook doesn't change anything, just calling addChangeset breaks the href attribute.

After

It still does, but it's now an ng-href which doesn't have the problem, which the angular docs suggest using for other reasons too.

Technical Details

See lab ticket. I don't think this is workaroundable in a consistent way in Civi\Angular\Coder so that you could still use regular href.

Comments

Has test, sort of.

@civibot
Copy link

civibot bot commented May 21, 2023

No issue was found matching the number given in the pull request title. Please check the issue number.

@civibot
Copy link

civibot bot commented May 21, 2023

(Standard links)

@civibot civibot bot added the master label May 21, 2023
@colemanw colemanw merged commit 3a32095 into civicrm:master May 21, 2023
@colemanw
Copy link
Member

This is a decent workaround

@demeritcowboy demeritcowboy deleted the nghref branch May 21, 2023 15:49
@demeritcowboy
Copy link
Contributor Author

ngThanks!

@colemanw
Copy link
Member

@demeritcowboy maybe we should replace all href="{{...}}" with ng-href in core, to set a good example if nothing else.

@demeritcowboy
Copy link
Contributor Author

Agreed. I left the ticket open - I was going to try to come up with some clever grep.

@colemanw
Copy link
Member

Looks like @stesi561 beat you to it @demeritcowboy

@demeritcowboy
Copy link
Contributor Author

ngYep

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.

2 participants