D19624: [T4538] Collapsable cells

[Nikita Sirgienko]

sirgienko marked 2 inline comments as done.
sirgienko added inline comments.

INLINE COMMENTS

> asemke wrote in commandentry.cpp:283
> do you really need this new variable m_collapsedResultCount here? You're clearing m_resultsItems anyway. Why not to check like
> 
>   if (m_expression->results().size())
>   {
>       if (m_resultItems.size())
>           //result(s) available and result item(s) shown -> allow to hide
>       else
>           //result(s) available but result item(s) hidden  -> allow to show
>   }
> 
> ?

I think, we really need this variable in `updateEntry`. `updateEntry` called, when we redraw worksheet or when we have new result. So, `m_collapsedResultCount`, stored count of collapsed results, needed to distinguish the first situation from the second.

> asemke wrote in commandentry.cpp:1180
> won't we run into problems with the performance for rendered image results for example? Do we really need to remove items and create them again? Why not to simple hide/show them without deleting them?

I agree, this is not very effective. But, I can't found another solution. I have tried show/hide; disable/enable, but results item in this cases don't free their places, so we have empty space between command entries. So, I open to suggestions

REPOSITORY
  R55 Cantor

REVISION DETAIL
  https://phabricator.kde.org/D19624

To: sirgienko, asemke
Cc: kde-edu, asemke, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20190308/8a37e2c3/attachment.html>