ahmadsamir added a comment.
I forgot to change the commit message about hiding the show only monospaced
fonts checkbox when FixedFontsOnly is set... at least the API docs do mention
it :/.
REPOSITORY
R236 KWidgetsAddons
REVISION DETAIL
https://phabricator.kde.org/D29065
To:
This revision was automatically updated to reflect the committed changes.
Closed by commit R236:0004e5c89a24: [KFontChooser] Add new DisplayFlag; modify
how flags are used (authored by ahmadsamir).
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
ahmadsamir marked an inline comment as done.
REPOSITORY
R236 KWidgetsAddons
BRANCH
l-kfontchooser-onlyfixed-display-flag (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D29065
To: ahmadsamir, #frameworks, dfaure, cfeck, bport
Cc: kossebau, kde-frameworks-devel,
ahmadsamir updated this revision to Diff 81007.
ahmadsamir marked an inline comment as done.
ahmadsamir added a comment.
More docs, and a trailing comma in the enum to ease git diff/blame if more
flags are added
REPOSITORY
R236 KWidgetsAddons
CHANGES SINCE LAST UPDATE
dfaure added a comment.
I agree, it's now common practice to have a final comma.
REPOSITORY
R236 KWidgetsAddons
BRANCH
l-kfontchooser-onlyfixed-display-flag (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D29065
To: ahmadsamir, #frameworks, dfaure, cfeck, bport
Cc:
kossebau added inline comments.
INLINE COMMENTS
> ahmadsamir wrote in kfontchooser.h:90
> Wouldn't that look a bit weird as if something has been removed or missing?
I might perhaps while getting used to if you are coming from old code habits,
but the syntax of C++ has been extra extended to
ahmadsamir marked an inline comment as done.
ahmadsamir added inline comments.
INLINE COMMENTS
> dfaure wrote in kfontchooser.h:87
> maybe document that FixedFontsOnly implies NoFixedCheckBox?
>
> (as in, the widget will behave as if NoFixedCheckBox was set)
Yes, will do (less surprises for
kossebau added inline comments.
INLINE COMMENTS
> kfontchooser.h:90
> +ShowDifferences = 4,///< Display the font differences interfaces
> +NoFixedCheckBox = 8 ///< Do not Show a checkbox to toggle
> showing only fixed fonts
> };
I would add a final ",", so you do
dfaure added inline comments.
INLINE COMMENTS
> kfontchooser.h:87
> +NoDisplayFlags = 0, ///< No flags set
> +FixedFontsOnly = 1, ///< Only show fixed fonts, excluding
> proportional fonts
> +DisplayFrame = 2, ///< Show a visual frame around the chooser
dfaure accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R236 KWidgetsAddons
BRANCH
l-kfontchooser-onlyfixed-display-flag (branched from master)
REVISION DETAIL
https://phabricator.kde.org/D29065
To: ahmadsamir, #frameworks, dfaure, cfeck, bport
Cc:
ahmadsamir updated this revision to Diff 80989.
ahmadsamir marked 2 inline comments as done.
ahmadsamir retitled this revision from "[KFontChooser] Add a DisplayFlag to
allow not showing fixedOnly checkbox" to "[KFontChooser] Add new DisplayFlag;
modify how flags are used".
ahmadsamir edited the
11 matches
Mail list logo
