Bug 139512 - Insert table in Impress insert a huge table (cell size). It's was more decently proportioned in the past
Summary: Insert table in Impress insert a huge table (cell size). It's was more decent...
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
5.1.0.3 release
Hardware: All All
: medium normal
Assignee: Justin L
URL:
Whiteboard: target:7.6.0
Keywords: bibisected, bisected, difficultyBeginner, easyHack
Depends on:
Blocks: ImpressDraw-Tables
  Show dependency treegraph
 
Reported: 2021-01-09 15:02 UTC by Telesto
Modified: 2023-05-20 18:10 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
Bibisect log (2.91 KB, text/plain)
2021-01-09 16:08 UTC, Telesto
Details
Bibisect log 5.0 (3.23 KB, text/plain)
2021-01-09 16:37 UTC, Telesto
Details
PDF export 6x4 table 4472 (desired.Expected) (6.58 KB, application/pdf)
2021-01-09 17:52 UTC, Telesto
Details
PDF export 6x4 table created with 7200 (default size) (6.54 KB, application/pdf)
2021-01-09 17:53 UTC, Telesto
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2021-01-09 15:02:49 UTC
Description:
Insert table in Impress insert a huge table (cell size). It's was more decently proportioned in the past

Steps to Reproduce:
1. Open Impress
2. Insert table with picker

Actual Results:
Large cells

Expected Results:
Smaller cells similar to writer table (and same as in LibreOffice 5.0)


Reproducible: Always


User Profile Reset: No



Additional Info:
Found in
7.2

6.2

and in
Versie: 5.1.6.2 
Build ID: 07ac168c60a517dba0f0d7bc7540f5afa45f0909
CPU Threads: 4; Versie besturingssysteem:Windows 6.2; UI Render: GL; 
Locale: nl-NL (nl_NL); Calc: CL

but no in
5.0
Comment 1 Telesto 2021-01-09 16:08:03 UTC
Created attachment 168793 [details]
Bibisect log

Bisected to

author	Justin Luth <justin_luth@sil.org>	2016-01-06 21:23:47 +0300
committer	Samuel Mehrbrodt <Samuel.Mehrbrodt@cib.de>	2016-01-07 09:11:10 +0000
commit 66b8b09d0aa97e4009d2019a9d93b74cd1e11a09 (patch)
tree 2858c6ef5b39d5b495f3017e90d8b0aa3779c6dc
parent 184a6a5301d699e0e1b3c16ddb27afc34e17a9d7 (diff)
set reasonable default size for new draw/impress tables
The width was decent, except that on small paper (A6) it was
wider than the paper size.

The height was terrible - the minimum size possible. It was too small
for even a single line of text. That made it hard to grab the table edge
(instead grabbing a row edge) when trying to resize.

Now the height and width are limited to the page/view size,
and based on the number of rows created.  One possible enhancement
would be to use the border width instead of the page width.

https://cgit.freedesktop.org/libreoffice/core/commit/?id=66b8b09d0aa97e4009d2019a9d93b74cd1e11a09
Comment 2 Telesto 2021-01-09 16:25:47 UTC
For what's worth.. the bibisected commit commit did make a change from insanely small table to unexpected large :-).. The 'proper size' got lost somewhere else
Comment 3 Telesto 2021-01-09 16:37:53 UTC
Created attachment 168795 [details]
Bibisect log 5.0

Original issue appeared with

author	yogesh.bharate001 <yogesh.bharate@synerzip.com>	2015-03-20 20:37:52 +0530
committer	Caolán McNamara <caolanm@redhat.com>	2015-05-12 09:26:38 +0000
commit 4f2c8194f485b1527fb4f4dfe23ce804937f1f9c (patch)
tree eb366db4c08d492c339a31a3394e2de4896d9b73
parent 7ff58e1a8965606a9fb45153a377b84593746420 (diff)
tdf#80340: Table changes format in PPTX format
Problem:
- If the PPTX contains embedded table i.e copied from excel, when we open it in
impress it row height increase due to this table format changes.
- Table contents empty row i.e without text, then row height increase because
text height is added also added for empty row.

Solution:
- Added check whether row contents text or not.

https://cgit.freedesktop.org/libreoffice/core/commit/?id=4f2c8194f485b1527fb4f4dfe23ce804937f1f9c
Comment 4 Telesto 2021-01-09 16:43:43 UTC
@Justin
Does the current self now, why the prior self picked such a size?
The table kind of overdimensioned at inserting.. 

So I kind of disagree with "reasonable default". It was by far to small at the point you fixed it, but this kind of the opposite (to large)

I prefer the 'original' old state.. prior to 4f2c8194f485b1527fb4f4dfe23ce804937f1f9c 

Lacking a clue if an UX opinion is needed.. 5 years ago...

And well.. who broke it.. synerzip.com.. The did a 'great job' [but well no clue about the amount of commit the did.. so I might be to sarcastic about that]
Comment 5 Justin L 2021-01-09 16:55:22 UTC
I think you will have to include some better reproduction steps with a screenshot, or else an impress example, because I really have no idea what you are trying to say. A 5x2 table looks perfect to me.

Plus, once the content is in the table, there are several "size" options to nicely minimize or optimize the table.
Comment 6 Telesto 2021-01-09 17:52:17 UTC
Created attachment 168798 [details]
PDF export 6x4 table 4472 (desired.Expected)
Comment 7 Telesto 2021-01-09 17:53:02 UTC
Created attachment 168799 [details]
PDF export 6x4 table created with 7200 (default size)
Comment 8 Telesto 2021-01-10 08:36:50 UTC
Initial report was based on me 'disliking' it, but lets try to give some arguments


* Table size isn't consistent.. Take maximum number of rows the table picker allows (in toolbar).. cells are smaller compared to 5 rows
* The insert table in maximum amount of rows overlapping the 'Title area'. Not thing you mostly want. You like only need 5 rows to achieve that
* The square suggests rather large content.. which isn't the case
* The size of the table cells kind of unconventional
* The table has fit to table content size feature.. why expand those cells in advance.. you have to downsize again
* I personally find those square size ugly. I originally thought there was some fundamental idea behind it. But well appears to be more circumstantial coincidence

And there are few other bugs which make working with tables in Impress harder (getting the outer cell.. or being able to resize top or bottom row).. however those are actual (different) bugs.. 
However if the size would be 'normal' the bugs related to resizing would be obvious. More an 'additional' circumstantial argument fitting my plea..
Comment 9 Heiko Tietze 2021-01-19 08:55:55 UTC
I agree with Telesto. The default row height has room for two lines of text with 18pt and some pixels margin to the bottom. Looks totally wrong unless you make it smaller and vertically center the content.

Besides, there is way to precisely adjust the row height. Optimal doesn't work for me and I cannot find any other function. So it's hard to give an advice with 1cm height, for example. An option could be to make it as small as possible and size per content, ie. paragraph with 1.5 lines spacing or so. But in this case I get a nice distance from top but not bottom.
Comment 10 Justin L 2021-01-19 11:46:09 UTC
Most of the time in Impress, the slide has a "table" option, and then it follow another if( pPickObj ) code path - even if you use Insert - Table. That is why I could not reproduce earlier. So use Draw to reproduce this, or else a content-only slide.

The fix of just setting a different starting value is easyhack beginner. A more robust way would be to base the row height on the font size of the table contents. (Currently I assume this is hard-coded at 18, since I don't notice any style change affecting it.)

Trying to scientifically determine the best default size is left to the reader as an exercise (because I don't know).

Note that a change here also affects Draw.
Comment 11 Justin L 2023-05-20 12:29:06 UTC
(In reply to Telesto from comment #4)
> I prefer the 'original' old state.. prior to
> LO 5.0 4f2c8194f485b1527fb4f4dfe23ce804937f1f9c 
Ah yes. That commit started a massive train-wreck. This caused empty rows to not have any say on row height.  Fixed by Sarper in 7.4.2. See bug 144092.

Since that is fixed, the minimum height (based on the font connected to that cell) will be correct. (Well, actually for a new table was correct since 5.1.1). So we can revert back to a rather minimalistic height again - like the original 200 (appropriate for 4pt font).

In practice the actual height will typically be ~750/row. That is from 18pt text plus 0.5pt * 2 borders + 0.4cm * 2 border spacing. (72pt == 1 inch).
So 2.54/72 = 0.30 cm/pt * 19pt = .67cm + .08cm = .75cm or 750 mm100.


(In reply to Telesto from comment #4)
> Does the current self know why the prior self picked such a size?
Not really, but since selecting rows and then using the context menu to size -> minimize row height is possible, I wasn't too worried about creating a two-line row height. At the time of my change, the height was guaranteed to fit inside of the viewing window, so the table could easily shrink and thus a large-ish value didn't seem bad. That guarantee changed soon after in 5.1.1.

The "best" size is likely 750, which is approximately the minimum amount needed for an 18pt font. (I don't seen any way to create a table that DOESN'T have a size 18 font - styles don't seem to affect the table?) But assuming that at some point the table size can be controlled, I decided to just revert back to 200 as the minimum height.

https://gerrit.libreoffice.org/c/core/+/152021
Comment 12 Commit Notification 2023-05-20 18:09:08 UTC
Justin Luth committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/fc15f483988622b8e3ea2a97c9a8593f2beb18fb

tdf#139512: partial revert default row height on new sd tables

It will be available in 7.6.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.