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/financial#171 - Don't #19876

Merged
merged 1 commit into from
Mar 23, 2021
Merged

Conversation

demeritcowboy
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/financial/-/issues/171

Before

Does

After

Doesn't

Technical Details

It's about money. The term markup can apply to the price of things. That's normal business, but it seems to be confused here with hypertext markup (html).

That's meant to be a pun - maybe a bit too involved.

Comments

This is a pretty generic form, so it may cause some fields to no longer display currency symbols or convert decimal separators.

@civibot civibot bot added the master label Mar 23, 2021
@civibot
Copy link

civibot bot commented Mar 23, 2021

(Standard links)

@eileenmcnaughton
Copy link
Contributor

I think we need to merge this since it's clearly wrong to pass html to the money format function & it should be formatted in the assigned html. I can that IS the case in the membership type form which uses this. We probably need to keep digging to find possible other places

The extra @mattwire added was 'Towards supporting EntityForm for 'View Action'

@mattwire can you identify any forms that implement that we can follow up & find a correct fix for money formatting for them 48c242d

@eileenmcnaughton
Copy link
Contributor

Merging per above but we need to look for possible fall out from this & the previous change in the 6 weeks before release

@eileenmcnaughton eileenmcnaughton merged commit 8ada914 into civicrm:master Mar 23, 2021
@mattwire
Copy link
Contributor

@eileenmcnaughton I don't remember this well but looks like it was to handle inconsistencies with the way "formatted" values are passed to the form. Agree it needs cleaning up.

@eileenmcnaughton
Copy link
Contributor

@mattwire well if you can think of what to test...

@demeritcowboy demeritcowboy deleted the dont branch March 24, 2021 00:04
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