Page 1 of 1

Issue 4440: Merge & Unify right-click lists on toolbar buttons

Posted: 04 Nov 2018, 16:04
by Sky
So I decided to pick up the issue.

So, in accordance to "How to contribute" instruction, I forked the repo, created branch for the issue and this thread on forum. Then I moved both mPanel and mTable along with showPanel method implementation from SceneToolTextureBrush and SceneToolRun to their base class SceneTool. I'm not sure this is how it should be done, though. As far as I can see SceneToolMode, SceneToolToggle, SceneToolToggle2 are also subclasses of SceneTool. And it seems they don't have to show such behavior (show the panel on RMB press).

Am I doing things right? Any hints? Thanks.

Re: Issue 4440: Merge & Unify right-click lists on toolbar buttons

Posted: 06 Nov 2018, 17:39
by Zini
The idea behind this was to create a new class for the list and then use this class in the button classes that need it.

Re: Issue 4440: Merge & Unify right-click lists on toolbar buttons

Posted: 09 Nov 2018, 16:14
by Sky
I see. Any hints how exactly it should be implemented?
What I can imagine is a ContextMenu class as subclass of QFrame with QTableWidget* as its member.
Is it appropriate? In case if it is, should I just expose the table via setter/getter (or as public?) or should I wrap the table in ContextMenu's methods?

Re: Issue 4440: Merge & Unify right-click lists on toolbar buttons

Posted: 12 Nov 2018, 14:57
by Zini
Sorry for the late reply. The last couple of days have been really busy for me.
What I can imagine is a ContextMenu class as subclass of QFrame with QTableWidget* as its member.
That sounds about right.
Is it appropriate? In case if it is, should I just expose the table via setter/getter (or as public?) or should I wrap the table in ContextMenu's methods?
The later please. Better more abstraction than less.