loh.tar added a comment.
Hi, I accidentally went by.
How about to add an icon to some (main) toolbar, if any is present?
That would the need to show such a message reduce to cases where no tool bar
is available. If that icon was added, and not already there, could that pop up
a small
lsartorelli updated this revision to Diff 46321.
lsartorelli added a comment.
Added @since and @author in the header documentation
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17013?vs=46248=46321
REVISION DETAIL
aacid added a comment.
the documentation is mising a @since 5.SOMETHING
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D17013
To: lsartorelli, ngraham, #frameworks, #kde_applications
Cc: aacid, cfeck, broulik, kde-frameworks-devel, michaelh, ngraham, bruns
lsartorelli added a comment.
Usually the message should be always shown, but I think there there are some
exceptions.
- for Kate the message is supposed to be off till its code base is upgraded
with the removal of ts own popup.
- for dolphin the message should be shown only if the
ngraham added a comment.
Looking better now. Do we really want to only optionally show the warning
with a bool argument? In general bool arguments to functions are discouraged
because they're not very readable. And if the whole point of this function is
to always show a consistent message,
lsartorelli updated this revision to Diff 46248.
lsartorelli added a comment.
Hi, you are right, I did not realize a bit of mess in between my revisions.
Now all the requests should have been fulfilled properly.
Thanks for this massive support and please, just let me know if I need
ngraham added a comment.
Even though a lot of inline comments have been marked as resolved, I don't
actually see that the requested changes have been made.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D17013
To: lsartorelli, ngraham, #frameworks,
lsartorelli marked an inline comment as done.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D17013
To: lsartorelli, ngraham, #frameworks, #kde_applications
Cc: aacid, cfeck, broulik, kde-frameworks-devel, michaelh, ngraham, bruns
lsartorelli updated this revision to Diff 45934.
lsartorelli marked an inline comment as done.
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17013?vs=45874=45934
REVISION DETAIL
https://phabricator.kde.org/D17013
AFFECTED FILES
src/CMakeLists.txt
aacid added inline comments.
INLINE COMMENTS
> lsartorelli wrote in ktoggleshowmenubaraction.cpp:77
> Do you suggest to add a warning in case of no shortcut?
> The only options I am thinking of are
> to tell the user add the missing shortcut via system settings,
> to tell the user to modify the
lsartorelli marked an inline comment as done.
lsartorelli added inline comments.
INLINE COMMENTS
> aacid wrote in ktoggleshowmenubaraction.cpp:77
> If shortcut() returns a QKeySequence (i think it does), you want
> toString(NativeText)
>
> Not a native speaker but i don't think "typing" is
lsartorelli updated this revision to Diff 45874.
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17013?vs=45821=45874
REVISION DETAIL
https://phabricator.kde.org/D17013
AFFECTED FILES
src/CMakeLists.txt
src/ktoggleshowmenubaraction.cpp
aacid added inline comments.
INLINE COMMENTS
> ktoggleshowmenubaraction.cpp:77
> +KMessageBox::information(d->window, tr("This will hide the menu bar
> completely."
> +" You can show it again by
> typing
lsartorelli updated this revision to Diff 45821.
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17013?vs=45812=45821
REVISION DETAIL
https://phabricator.kde.org/D17013
AFFECTED FILES
src/CMakeLists.txt
src/ktoggleshowmenubaraction.cpp
lsartorelli marked 3 inline comments as done.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D17013
To: lsartorelli, ngraham, #frameworks, #kde_applications
Cc: cfeck, broulik, kde-frameworks-devel, michaelh, ngraham, bruns
ngraham added inline comments.
INLINE COMMENTS
> ktoggleshowmenubaraction.h:57
> +/**
> + * Sets the parent window for message box with the user remainder.
> + * @param window the window that will be parent for the message box
remainder -> reminder
> ktoggleshowmenubaraction.h:64
ngraham added reviewers: Frameworks, KDE Applications.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D17013
To: lsartorelli, ngraham, #frameworks, #kde_applications
Cc: cfeck, broulik, kde-frameworks-devel, michaelh, ngraham, bruns
lsartorelli added inline comments.
INLINE COMMENTS
> cfeck wrote in ktoggleshowmenubaraction.h:28
> reminds
>
> Additionally, "show back" does not sound like proper english, but I am not a
> native english speaker.
Unfortunately, me too, ... any suggestion is appreciated
REPOSITORY
R236
lsartorelli updated this revision to Diff 45812.
lsartorelli marked 2 inline comments as done.
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17013?vs=45811=45812
REVISION DETAIL
https://phabricator.kde.org/D17013
AFFECTED FILES
src/CMakeLists.txt
lsartorelli updated this revision to Diff 45811.
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17013?vs=45807=45811
REVISION DETAIL
https://phabricator.kde.org/D17013
AFFECTED FILES
src/CMakeLists.txt
src/ktoggleshowmenubaraction.cpp
cfeck added inline comments.
INLINE COMMENTS
> ktoggleshowmenubaraction.cpp:21
> +
> +#include "ktoggleshowmenubaraction.h"
> +
Do we still follow the rule "include own headers first"?
> ktoggleshowmenubaraction.h:28
> + * An action to hide or show the menubar of a window.
> + * The action by
lsartorelli added a dependent revision: D17016: Show menu bar, how to
re-enable, common shortcut dialog.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D17013
To: lsartorelli, ngraham
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
lsartorelli marked 2 inline comments as done.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D17013
To: lsartorelli, ngraham
Cc: broulik, kde-frameworks-devel, michaelh, ngraham, bruns
lsartorelli updated this revision to Diff 45807.
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D17013?vs=45798=45807
REVISION DETAIL
https://phabricator.kde.org/D17013
AFFECTED FILES
src/CMakeLists.txt
src/ktoggleshowmenubaraction.cpp
broulik added inline comments.
INLINE COMMENTS
> ktoggleshowmenubaraction.cpp:26
> +
> +#define i18n QString::fromLatin1
> +
What's this for?
> ktoggleshowmenubaraction.h:49
> + */
> +KToggleShowMenuBarAction(QWidget *window, QObject *parent);
> +
I know we typically use `window` here
lsartorelli added a dependent revision: D17014: Show menu bar, how to
re-enable, common shortcut dialog.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D17013
To: lsartorelli, ngraham
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
lsartorelli created this revision.
lsartorelli added a reviewer: ngraham.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
lsartorelli requested review of this revision.
REVISION SUMMARY
Provide a common message ,encapsulated into the ShowMenuBarAction, with
27 matches
Mail list logo
