Truncate _source summary in doc table#9394
Conversation
Previously when rendering the _source column in the doc table we would inject the entire _source contents into the DOM and then "truncate" it by setting a max height on the element in CSS. Despite being hidden from view this content still had an impact on the browser's layout calculations. Even for modestly sized documents (e.g. makelogs) the impact was quite large (>200 ms). This commit actually truncates the content to a maxiumum size based on number of characters in the field names and values.
There was a problem hiding this comment.
Oh my, the table_row code is a very special snowflake (not your new code, but the whole contraption). I guess we could gain a lot of performance by just rewriting that as a normal directive with a simple template. The way it is written now it stores a lot of state in the DOM and manipulates the DOM structure repeatedly (e.g. emptying the element and re-adding children) which usually does not perform well. I'd like to give that a try some day (if nobody else has done it by then).
Aside from that I am wondering whether this is a "breaking" change from the user's perspective. After all, you were able to select and copy+paste the whole cell content before even if it was truncated.
How did you come up with the magic number 500? The cell truncation height is configurable but this would just cut off the content after 500 characters even if the user set a large maximum height.
Anyway, this is a great place to start improving the performance 😄 thank you for investing the time 👍
| sourceList.append(listPair); | ||
| sourceList.append(' '); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This still performs a lot of individual DOM manipulations. What about first calculating the number of elements to keep and then removing any additional children of sourceList in one operation (e.g. using the :gt(index) selector and .remove())?
There was a problem hiding this comment.
Since sourceList hasn't been injected into the document yet, is appending really an expensive operation? The browser won't need to calculate style, layout, etc until the template is compiled here, right?
There was a problem hiding this comment.
Yes, you're correct. 😊 Still, since you're optimizing for performance just pushing individual items onto the collection might be costly. Probably depends on how this is implemented internally in jQuery/jqLite.
|
Yeah it's a bit of a rube goldberg setup :) I considered a larger rewrite but it didn't seem like the risk/reward was good enough at the moment. I don't think we generally consider purely UI changes to be breaking changes, but it's a good point. What's your opinion @epixa? 500 just seemed like a good number for most normal screen sizes at regular font sizes. I'll make it configurable, it makes sense considering users can adjust the truncated height. |
| return text; | ||
| } | ||
|
|
||
| function truncateSourceSummary(sourceElement) { |
There was a problem hiding this comment.
Seems to me it might make more sense to have this logic here. Then you don't have to check if the "discover-table-sourcefield" class exists, nor deal with the DOM. What do you think?
There was a problem hiding this comment.
I thought about that, went back and forth a bit and I could still be convinced but here was my thinking:
The source formatter is a generic utility that could be used in lots of places, so while we want a truncated version in the doc table we might not want a truncated version everywhere the source formatter is used. Since it's the doc table that has specific requirements for the content, it's best suited to do the truncation.
But I just realized the current implementation won't work. Someone could theoretically specify a different field formatter for the _source field, and this logic would break because it expects a particular html structure.
I thought about defining a new content type, so in addition to text and html maybe we'd have something like html-summary that the doc table could request but I dunno, it feels a little hacky, html-summary isn't really a content type. Do you have any ideas? I'm not super familiar with the field formatter code
|
The more I think about this, the more I feel a change in design will be a better solution than trying to optimize what we have now. There isn't really a good way to truncate the source preview without making a breaking change to the user experience and if we're going to do that we might as well consider other options, like auto-selecting columns the way @lukasolson mentioned. So I'm going to close this for now. |
5 participants
Previously when rendering the _source column in the doc table we
would inject the entire _source contents into the DOM and then
"truncate" it by setting a max height on the element in CSS. Despite
being hidden from view this content still had an impact on the browser's
layout calculations. Even for modestly sized documents (e.g. makelogs)
the impact was quite large (>200 ms). This commit actually truncates
the content to a maxiumum size based on number of characters in the
field names and values.
(the below tests were performed with this PR applied on top of #9326 to reduce the noise from the issues fixed by that PR)
Before
After
