Allow multiple tables to be used in templates. #23
Conversation
|
Note: README should be update with an example. |
Codecov Report
@@ Coverage Diff @@
## master #23 +/- ##
==========================================
+ Coverage 98.06% 98.11% +0.04%
==========================================
Files 3 3
Lines 1033 1059 +26
==========================================
+ Hits 1013 1039 +26
Misses 14 14
Partials 6 6
Continue to review full report at Codecov.
|
|
@matcornic up ? |
| }, | ||
| CustomAlignement: map[string]string{ | ||
| "Price": "right", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
I think we should improve the test by adding the title (and check its presence)
| padding: 35px 0; | ||
| margin: 25px 0 25px 15px; | ||
| padding-top: 15px; | ||
| } |
There was a problem hiding this comment.
Is this change in css is really necessary ? I don't really want to change the default theme unless it's necessary, as it could be considered as a regression for some people.
There was a problem hiding this comment.
I am guessing the idea here was to indent for the caption. I would inject a margin-left style in the template when caption Title field exists.
|
Hello
Glad to hear back from you.
I finally opted for a simpler solution in my use case, but I am still open
to finish this PR.
Regarding the tests, I agree with you, and i'll make the change.
Regarding the changes in the default theme CSS, I did it because I feel
like it is a significant gain in readability, instead of having items
packed, but I guess this is debatable, and things like this are subject to
personnal tastes. I encourage you to try it and see for yourself if the
visual result with and without the change is worth it.
Regards,
William
2017-11-16 20:31 GMT+01:00 Mathieu Cornic <[email protected]>:
… ***@***.**** requested changes on this pull request.
Thanks a lot for your PR :), I just wrote some comments and it should be OK
Note : Github did weird things with code comparison, not easy to find your
changes ^^
------------------------------
In hermes_test.go
<#23 (comment)>:
> },
- CustomAlignement: map[string]string{
- "Price": "right",
+ Columns: Columns{
+ CustomWidth: map[string]string{
+ "Item": "20%",
+ "Price": "15%",
+ },
+ CustomAlignement: map[string]string{
+ "Price": "right",
+ },
},
I think we should improve the test by adding the title (and check its
presence)
------------------------------
In default.go
<#23 (comment)>:
> @@ -207,8 +207,11 @@ func (dt *Default) HTMLTemplate() string {
/* Data table ------------------------------ */
.data-wrapper {
width: 100%;
- margin: 0;
- padding: 35px 0;
+ margin: 25px 0 25px 15px;
+ padding-top: 15px;
+ }
Is this change in css is really necessary ? I don't really want to change
the default theme unless it's necessary, as it could be considered as a
regression for some people.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#23 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AGN7EUyKfKsFX-4tiYEdX4RxzGGewwUMks5s3I2AgaJpZM4Pl_KN>
.
|
|
Any news on this PR? would really come in handy. Or at least a workaround to achieve this. |
|
@nlandeker Feel free to take over the PR and address @matcornic comments. |
|
Any progress on this PR? |
|
This is a good feature and would be great if this was integrated, any updates on this ? |
|
This feature would be great. I even would think that the order of elements in the mail should be flexible and can be repeated like the email creator wants it .. |
|
Feel free to takeover the PR Daniel, I don't use this project anymore and I
will not make further changes, but this would require minimal work to
address the comments made by matcornic.
Le mar. 30 juin 2020 à 20:33, Daniel Pötzinger <[email protected]> a
écrit :
… This feature would be great.
I even would think that the order of elements in the mail should be
flexible and can be repeated like the email creator wants it ..
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#23 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABRXWENQRPJ3ZAQXKQVHK2DRZIVWLANCNFSM4D4X6KGQ>
.
|
|
Would also like this feature! |
| @@ -62,7 +63,7 @@ type Body struct { | |||
| Name string // The name of the contacted person | |||
| Intros []string // Intro sentences, first displayed in the email | |||
| Dictionary []Entry // A list of key+value (useful for displaying parameters/settings/personal info) | |||
| Table Table // Table is an table where you can put data (pricing grid, a bill, and so on) | |||
| Tables []Table // Tables is a list of tables where you can put data (pricing grid, a bill, and so on) | |||
There was a problem hiding this comment.
I would suggest keeping both fields as this is a breaking change. We can log a warning the Table field is deprecated and if you use it. The way to keep this from breaking everyone we can add code to programmatically add Table to the Tables slice, while also warning the user of the deprecation.
|
I forked this project and am working on it here: https://github.com/go-hermes/hermes Completed the multi table support, bumped go.mod to 1.22, and bumped all available versions. |
Hello,
Following my Issue #22 I have made some changes to allow multiples tables in the templates.
I also added the field
Titleto typehermes.Table. With HTML, a<caption>aligned to the left is used, when rendering to text, the intermediate HTML template uses a<span>before the table.Example with two tables, and a title on the first one.
TEXT
HTML
