-
-
Notifications
You must be signed in to change notification settings - Fork 321
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
Vertically merge cells showing account names and Total #2232
Conversation
500d3df
to
a98643b
Compare
I see there's one new commit here; maybe it also depends on #2226 ? |
a98643b
to
233dd77
Compare
Yes, it depends on #2226. I have rebased just now. |
cf8ce94
to
c7360bd
Compare
@thielema, what's the problem this is solving, and the visible effects ? |
c7360bd
to
bd82537
Compare
On Sun, 29 Sep 2024, Simon Michael wrote:
@thielema, what's the problem this is solving, and the visible effects ?
There is a tension between nice appearance and the opportunity for further
processing. Showing a row header once looks nice, but for further
processing it is better to repeat the header in every row. There was
recently the commit 9788a06 that replaced
repeated headers by single ones. This indicates for me that HTML shall
look nice but is not intended for further processing.
I first thought about adding an option for controlling repetition of row
headers. But then I thought that formats like CSV and FODS are for further
processing and HTML is for human inspection, thus decided to repeat
headers in the first two formats but not in HTML. In FODS we can have both
features at once: Row headers can be repeated but only the first one is
shown in a merged cell. The HTML equivalent of this is a merged cell, but
HTML does not store covered cells.
If you prefer explicit control I suggest adding two options:
…--replicate-row-header = auto|once|duplicate
--use-row-span - boolean option
can be combined with any setting for replicate-header
and affects formulas accessing covered cells.
|
I'm a bit lost/forgetful, so I have questions. I cleaned up some visual glitch with html output a few months back, with 9788a06. I think I also tweaked some row headings in csv output also, in another commit. Perhaps we should talk about html to start with. |
On Sun, 29 Sep 2024, Simon Michael wrote:
Perhaps we should talk about html to start with.
You're right that I haven't been thinking of it as something to be reused in other ways.
Btw. HTML output can also be re-used, because LibreOffice-Calc recognizes
HTML tables and import them as such. However, detection of numbers does
not work well. Thus, I'd prefer importing FODS or CSV to LibreOffice for
further processing.
What are the changes to html output you propose, compared to 1.40 ?
We have
$ hledger-1.40 -f hledger-journal balance --output-file=multibalance.html --layout=bare --yearly
It shows:
Total: Euro
<blank> USD
Whereas hledger from thielema/header-row-span shows:
Total: Euro
<covered> USD
In 1.40, "Total" and "<blank>" are two separate cells.
New is, "Total" and "<covered>" are one merged cell. "<covered>" is not
shown.
You would see the difference if there were borders. In 1.40 there would be
a line between "Total" and "<blank>". In the merged cell there would be no
line.
|
And you've left csv output as-is (with the header repeated in each row, for ease of machine processing). And you've done something fancier with fods. Sounds great. |
On Sun, 29 Sep 2024, Simon Michael wrote:
Thanks, that got me closer. I'm with you now:
Screenshot.2024-09-29.at.08.27.55.png (view on web)
becomes
Screenshot.2024-09-29.at.08.28.13.png (view on web)
The first two columns lost their alignment, let's fix that.
Interesting. The only difference between old and new output is that the
cells got `valign="top"` attributes. This is important for vertically
merged cells, but I expected that it has no effect on atomic cells.
Surprisingly, valign also affects horizontal alignment.
|
bd82537
to
55ea279
Compare
…ly merge cells showing account names and Total lib: Write.Spreadsheet: add support for cell spans
55ea279
to
64224c4
Compare
On Mon, 30 Sep 2024, Simon Michael wrote:
The headings are now center aligned - it might be ok, was it intended ?
Now I see. It was not valign=top that made the difference.
The change came with commit
thielema@ff397f7#diff-eaee0cecc4187da2292f3659e7656b09b43a00e14759d40732b03a89d094b76a.
I had thrown away multiBalanceReportHtmlHeadRow,
multiBalanceReportHtmlBodyRow that contained some custom alignment styles.
I instead relied on default alignments. If you prefer the original
alignments I can add them, again.
|
I've seen advice that center-aligned headings are often best, but I probably preferred the content-aligned headings at some point in the past. Let's leave it for a bit and see how it feels! |
@@ -376,7 +376,7 @@ compoundBalanceReportAsHtml ropts cbr = | |||
Total simpleDateSpanCell totalrow | |||
-- make a table of rendered lines of the report totals row | |||
& map (map (fmap wbToText)) | |||
& zipWith (:) (Spr.defaultCell "Net:" : repeat Spr.emptyCell) | |||
& addRowSpanHeader (Spr.defaultCell "Net:") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@thielema: At some point, this cell lost its "account" class, causing "Net" to no longer be left-aligned. I didn't see how exactly to fix it myself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By the way you may have seen this already but: adding td, th {border: thin solid red;}
in a hledger.css
in the current directory is quite handy for checking these HTML reports.
I think this is the best compromise for multibalance:
Same applies to account names in
tidy
layout.