Introduce custom event reminder#6989
Conversation
f01a67f to
a931ead
Compare
| background-color: white; | ||
| display: block; | ||
| min-width: 250px; | ||
| min-width: 700px; |
There was a problem hiding this comment.
How likely is it that this does not break the display in some other place...? 😱
There was a problem hiding this comment.
It was a bad idea, indeed. Unfortunately the preview looks often silly with the auto size.
There was a problem hiding this comment.
If needed, the ajaxDialog has an dialogClasses argument which can be used to add a custom CSS class to the parent element of the exclusivePopup
| <hr> | ||
| <a href="{% block footer_url %}{{ url_for_index(_external=true) }}{% endblock %}">Indico</a> | ||
| :: {% block footer_title %}{% trans %}Email Notifier{% endtrans %}{% endblock %}<br> | ||
| <p><a href="{% block footer_url %}{{ url_for_index(_external=true) }}{% endblock %}">Indico</a> :: {% block footer_title %}{% trans %}Email Notifier{% endtrans %}{% endblock %}</p> |
There was a problem hiding this comment.
Why? Also it's much less readable now with such a long lone...
indico/modules/events/reminders/templates/emails/event_reminder.html
Outdated
Show resolved
Hide resolved
5096942 to
9774b99
Compare
9774b99 to
5d601ed
Compare
It does not seem particularly useful anywhere else
e5cf2cd to
65a8dc0
Compare
Otherwise e.g. links are simply lost, which is not great.
- add missing css to html email preview - use editor-output styling for preview dialog
The subject isn't HTML, tags are fine there
5306aee to
95add1c
Compare
| self.message.validators.append(DataRequired()) | ||
| self.message.flags.required = True |
There was a problem hiding this comment.
FYI this is incorrect, I also missed it when reviewing it - when you modify the validators it leaks into every other instance of that form within the same Python process.
The correct way to do this is using our inject_validators util. I fixed it in fe1b369
There was a problem hiding this comment.
Yes, I noticed that yesterday and tried fixing it when working on #7115. I guess that grabbed your eyes to this problem :)
Thanks for fixing that I didn't know about that utility function which seems pretty robust.
This PR is about further improvements on event reminders
Implementation details:
I followed your advise regarding using
RenderModeMixinfor the message field. Old reminders got theRenderMode.plain_textand new reminders getsRenderMode.html. The migration doesn't translate existingtext/plainreminders totext/html. So at the end there are 3 types of reminder:In fact, the newly introduced
ReminderTypeenum is only standard or customized. I didn't want to embed the legacy one.