[Konsole-devel] Review Request: The context menu should contain the "Show Menubar" action when the menubar is hidden

[Jekyll Wu]

> On March 9, 2012, 3:36 p.m., Kurt Hindenburg wrote:
> > Since the loop is only when showing the shortcut dialog, can you just check for dups in the loop?
> > 
> > I don't like the 'show menubar' at the bottom.  Can you add it in the same position as it is now?

"Since the loop is only when showing the shortcut dialog, can you just check for dups in the loop?"

I tried, but it didn't work. I know and can detect the duplication in the code, but the problem is there is no way to tell KShortcutsDialog : "ignore duplicate".

I even tried removing "show-menubar" manually from the actionCollection() of SessionController, but the result is crash when configuring shortcut.


"I don't like the 'show menubar' at the bottom.  Can you add it in the same position as it is now?"

That would make the code more coupled with the exact layout of the context menu. If we later adds or removes another action before that "Show Menubar" action, this part of code should also be updated. 


- Jekyll


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/104193/#review11254
-----------------------------------------------------------


On March 8, 2012, 6:42 a.m., Jekyll Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104193/
> -----------------------------------------------------------
> 
> (Updated March 8, 2012, 6:42 a.m.)
> 
> 
> Review request for Konsole.
> 
> 
> Description
> -------
> 
> The cause of that bug is "show-menubar" is contained in both konsoleui.rc(used by MainWindow) and sessionui.rc(used by SessionController). In MainWindow::showShortcutsDialog(), the following code adds that 'show-menubar' action twice since it is contained in the 'actionCollection()' of both MainWindow and SessionController.
> 
>     foreach(KXMLGUIClient * client, guiFactory()->clients()) {
>         dialog.addCollection(client->actionCollection());
>     }
> 
> The patch removes "show-menubar" from sessionui.rc. Instead, it adds or removes that action into/from context menu dynamically depending upon whether the window currently has menubar or not. The result is the content menu contains that "Show Menubar" action only when the menubar is hidden.
> 
> I feel this is not an ideal solution. Is there some way to tell KShortcutsDialog: "If the same action is added twice, Just count it as one single action"? That would make the code easier . 
> 
> And I'm not sure appending the "Show Menubar" action is nice or ugly. Should a separator be added before it?
> 
> 
> This addresses bug 214493.
>     http://bugs.kde.org/show_bug.cgi?id=214493
> 
> 
> Diffs
> -----
> 
>   desktop/sessionui.rc 0dd1b66 
>   src/SessionController.h 46714c0 
>   src/SessionController.cpp f437d9b 
> 
> Diff: http://git.reviewboard.kde.org/r/104193/diff/
> 
> 
> Testing
> -------
> 
> seems fine in stand-alone konsole and hosts of konsolepart.
> 
> 
> Screenshots
> -----------
> 
> when menubar is visible
>   http://git.reviewboard.kde.org/r/104193/s/452/
> when menubar is hidden
>   http://git.reviewboard.kde.org/r/104193/s/453/
> 
> 
> Thanks,
> 
> Jekyll Wu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/konsole-devel/attachments/20120309/2c7ac2c6/attachment.html>