[Okular-devel] Review Request 127013: Properly show marker for current section in TOC

[Albert Astals Cid]

> On Feb. 29, 2016, 11:52 p.m., Albert Astals Cid wrote:
> > I was going to commit this but then i realized I can't because it will crash the mobile version. In documentitem.cpp the parent of the model is not a view.
> > 
> > This highlights a problem with the patch, it accesses the view from the model, that is wrong, ideally models don't know about their views.
> > 
> > My suggestion to workaround this problem is this: 
> > * Leave the decoration role as it is  (as deprecated for the mobile version)
> > * Introduce a new role named someting like PathToLeaf that returns the arrow item for the whole path (except the leaf) (for that we do not need view info)
> > * In pageitemdeleate (that is tied to the view) you can inspect the view expanded/collapsed status + the new role to decide whether to draw the triangle or not for the non leaf
> > 
> > Does that sound doable?
> 
> Albert Astals Cid wrote:
>     Stupid markdown processing, i meant
>      * Leave the decoration role as it is (as deprecated for the mobile version)
>      * Introduce a new role named someting like PathToLeaf that returns the arrow item for the whole path (except the leaf) (for that we do not need view info)
>      * In pageitemdeleate (that is tied to the view) you can inspect the view expanded/collapsed status + the new role to decide whether to draw the triangle or not for the non leaf
> 
> Miklós Máté wrote:
>     I can't compile the kactive code, because there is a dependency conflict between libkactivities-dev 4.13 and kde 5. Even if I could compile it, I have no idea how I could run it.
>     
>     I have an idea for the solution, though. How about "dynamic_cast< QTreeView* > ( QObject::parent() );" and only filter the highlighted items when this returns not NULL?
> 
> Albert Astals Cid wrote:
>     > I can't compile the kactive code, because there is a dependency conflict between libkactivities-dev 4.13 and kde 5.
>     
>     There's no such thing as KDE 5 ;)
>     
>     
>     > Even if I could compile it, I have no idea how I could run it.
>     
>     Well it creates a binary, is not hard at all
>     
>     
>     > I have an idea for the solution, though. How about "dynamic_cast< QTreeView* > ( QObject::parent() );" and only filter the highlighted items when this returns not NULL?
>     
>     That's a bit of a hack, what's wrong with the solution i suggested that properly applies the model/view separation?
> 
> Miklós Máté wrote:
>     > That's a bit of a hack, what's wrong with the solution i suggested that properly applies the model/view separation?
>     
>     The model and the view are already entangled, see TOCModel::fill(). I'll look into the PageItemDelegate, but I think the current solution is much simpler than custom drawing.
>     
>     BTW how do you quote the previous message in your reply?

> The model and the view are already entangled, see TOCModel::fill(). I'll look into the PageItemDelegate, but I think the current solution is much simpler than custom drawing.

Ok, if you can make it so that the tocmodel only changes behaviour if the parent is a treeview i guess we can try to go with that

> BTW how do you quote the previous message in your reply?

See the Markdown reference link, but basically it's start the sentence by ">" and then two newlines for your answer


- Albert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127013/#review92956
-----------------------------------------------------------


On Feb. 27, 2016, 10:30 a.m., Miklós Máté wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127013/
> -----------------------------------------------------------
> 
> (Updated Feb. 27, 2016, 10:30 a.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> -------
> 
> fixes bug #342076
> 
> 
> Diffs
> -----
> 
>   ui/tocmodel.cpp ce93366 
> 
> Diff: https://git.reviewboard.kde.org/r/127013/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> okular_marker_v2.diff
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/02/27/78357c93-a723-4e1b-8a8d-638d49ed334e__okular_marker_v2.diff
> okular_marker_v2.diff
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/02/27/c4395f9d-2991-4f6c-bf36-d973dbcb5b64__okular_marker_v2.diff
> 
> 
> Thanks,
> 
> Miklós Máté
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20160309/1646b9eb/attachment-0001.html>