D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

ahmadsamir added a comment. In D28774#676000 , @volkov wrote: > Why not add support for `QPlatformTheme::FontDialog` in `KdePlatformTheme::createPlatformDialogHelper()`? Sorry about the late reply; I had looked into the platformtheme inte

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

volkov added a comment. Why not add support for `QPlatformTheme::FontDialog` in `KdePlatformTheme::createPlatformDialogHelper()`? REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D28774 To: ahmadsamir, #frameworks, dfaure, cfeck Cc: volkov, kde-frameworks-devel

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

ahmadsamir added a comment. In D28774#647157 , @dfaure wrote: > I mean, the user can press cancel, that's when QColorDialog returns an invalid color. > If there was a concept like an invalid font we could do the same here, but there's no such

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

dfaure added a comment. I mean, the user can press cancel, that's when QColorDialog returns an invalid color. If there was a concept like an invalid font we could do the same here, but there's no such concept in QFont. So yep, no better way. REPOSITORY R236 KWidgetsAddons REVISION DET

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

ahmadsamir added inline comments. INLINE COMMENTS > ahmadsamir wrote in kfontrequester.cpp:188 > Plus we can't end up with a non-valid QFont, the dialog is filled from > QFontDataBase; so unless the user went and uninstalled the font while the > dialog was open, it should be OK. Also, font subs

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

ahmadsamir added inline comments. INLINE COMMENTS > dfaure wrote in kfontrequester.cpp:188 > It's really not obvious from the above that this modifies m_selFont. > > But I see QFont has no isValid() so we can't adopt an API like > QColorDialog::getColor (plus KFontChooserDialog has been release

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

This revision was automatically updated to reflect the committed changes. Closed by commit R236:56090c8f380e: [KFontRequester] Port from QFontDialog to KFontChooserDialog (authored by ahmadsamir). REPOSITORY R236 KWidgetsAddons CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D28774?vs=

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

dfaure accepted this revision. dfaure added inline comments. This revision is now accepted and ready to land. INLINE COMMENTS > kfontrequester.cpp:188 > + > +const int result = KFontChooserDialog::getFont(m_selFont, flags, > q->parentWidget()); > It's really not obvious from the above tha

D28774: [KFontRequester] Port from QFontDialog to KFontChooserDialog

ahmadsamir created this revision. ahmadsamir added reviewers: Frameworks, dfaure, cfeck. Herald added a project: Frameworks. ahmadsamir requested review of this revision. TEST PLAN The kfontrequestertest app still works REPOSITORY R236 KWidgetsAddons BRANCH l-kfontreq (branched from master