D29065: [KFontChooser] Add a DisplayFlag to allow not showing fixedOnly checkbox

[Ahmad Samir]

ahmadsamir added inline comments.

INLINE COMMENTS

> dfaure wrote in kfontchooser.h:90
> 1/2/4 looked like powers of 2, i.e. a flag set.
> 
> Using 5 here gives is the same value as FixedFontsOnly | ShowDifferences.
> 
> On the other hand it makes sense FixedFontsOnly would automatically hide the checkbox, no? If the app says "fixed fonts only", the user has no choice in the matter.
> 
> I'm also confused by the name "No[...]Toggle" and the docu saying this will show a checkbox. Something's off by "not" ;)
> Sounds wrong?

> 1/2/4 looked like powers of 2, i.e. a flag set.
>  Using 5 here gives is the same value as FixedFontsOnly | ShowDifferences.

I missed the fact that combining two or more flags would give the same value... will fix.

> On the other hand it makes sense FixedFontsOnly would automatically hide the checkbox, no? If the app says "fixed fonts only", the user has no choice in the matter.

Yeah, that makes sense (a special case is Konsole, which embeds KFontChooser and adds a checkbox to "show all fonts", IIRC this is to offer users a way out with some mis-configured fonts... etc).

> I'm also confused by the name "No[...]Toggle" and the docu saying this will show a checkbox. Something's off by "not" ;)
>  Sounds wrong?

I wrote the enum docs, _then_ changed the name of the member to NoFixedOnlyToggle (this way apps can set it if they want but it doesn't have to be set by default, i.e. we always show the box unless told otherwise). So yeah off by "Do not".

REPOSITORY
  R236 KWidgetsAddons

REVISION DETAIL
  https://phabricator.kde.org/D29065

To: ahmadsamir, #frameworks, dfaure, cfeck, bport
Cc: kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200422/eb59e3dc/attachment.html>