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

Wysiwyg support for i18n event field - Confirmation Email Text #20922

Closed
wants to merge 2 commits into from

Conversation

monishdeb
Copy link
Member

@monishdeb monishdeb commented Jul 21, 2021

Overview

Add Wysiwyg text editor for "Confirmation Email Text"

BEFORE

before

AFTER

after

Followup of #14384

ping @colemanw @seamuslee001

@civibot civibot bot added the master label Jul 21, 2021
@civibot
Copy link

civibot bot commented Jul 21, 2021

(Standard links)

@mlutfy
Copy link
Member

mlutfy commented Jul 21, 2021

Hi Monish, is this the same as #13976 ? Could you provide before/after information?

Personally I would be very much in favour of having WYSIWYG for this field, but I think we have to handle upgrades?

From the other PR, Eileen had commented:

does this change how the text is saved (ie. add html tags to it). I feel like we tried something like this before & there was some gotchas

@eileenmcnaughton
Copy link
Contributor

I note that this field is in event_offline_receipt_text.tpl & currently we would not save the

markers. If we switch to rich text we will do so & they will look 'odd' - in the html version we see

  {if !empty($event.confirm_email_text) AND (empty($isOnWaitlist) AND empty($isRequireApproval))}
     <p>{$event.confirm_email_text|htmlize}</p>
    {/if}

Note that the upgrade script in #20867 is designed to update specific strings where a token has changed. The different between this and the main template upgrade script is it will assume the specific string should always be swapped & do it on customised as well as default templates (which makes sense here I think)

@mlutfy
Copy link
Member

mlutfy commented Sep 6, 2021

Related: https://lab.civicrm.org/extensions/richtext/-/tree/master

(although I still support changing this in core, it's a frequent request)

@demeritcowboy
Copy link
Contributor

This has merge conflicts and hasn't been worked on in several months, so going to close but feel free to reopen.

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.

4 participants