QtCurve already has an option for the popup opening delay, we should
have one for the auto-close case. Especially as Qt's default is weirdly
long.
Details
Details
- Reviewers
rjvbb yuyichao - Commits
- R626:a249871e95d4: Add config option for SloppyMenuClose delay
Diff Detail
Diff Detail
- Repository
- R626 QtCurve
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
As a general rule of thumb new configuration options should be added to the Qt4 branch too. At least as far as reading and writing them is concerned, but ideally also in the UI (with a note/tooltip if it concerns something Qt4 or Qt5 specific).
Rationale: QtCurve has a shared config file. It is also the "official" style for the KDE4 packages in MacPorts, which includes a stripped-down version of kde-workspace providing the KCMs and systemsettings utility. IOW, there's a class of users who are used to configuring QtCurve through the KDE4 systemsettings utility, on Mac (and possibly on older Linux distros that still provide a Plasma4 desktop but also Qt5/KF5 applications).
Someone ought to put the whole config code in a shared directory, refactored to build against Qt4 and Qt5... I've been thinking of doing that but never getting around to it.
Edit: I think this might interfere with my own D5286 RR; could we get that one done with first to avoid unnecessary rebasing?
Comment Actions
That's not a problem: I'm not asking to *implement* the feature, just to support reading, writing and preferably controlling it through the prefs files and UI. Otherwise you'd lose the setting when you change something in the Qt4 KCM!
Comment Actions
I would, but this is new in Qt5.5
You don't have to implement it, though it should be added to the loader/writer so that it can be maintained. This will also help to make sure different versisons are kept in sync since the " put the whole config code in a shared directory, refactored to build against Qt4 and Qt5" (and also gtk2 though it's less relavant now) is what I wanted to do all along = = ....................
Since this only has a effect on Qt5 the config GUI should also indicate that (either tooltip or description should be ok).
Comment Actions
I'm a bit sorry for asking, I know from experience how tedious and uninteresting this is (though I myself usually end up backporting tweaks to the Qt4 version as I still use lots of Qt4 apps, see the linked RR) :)
Comment Actions
Another question we should ask ourselves is if we want to impose requiring Qt 5.5 instead of still supporting Qt 5.3.2 . For KF5 users this won't make a difference but it does make a potential difference for the "pure Qt" version of the style. There are still distributions that don't offer Qt 5.5 and idem on Mac there are OS versions that cannot run anything beyond 5.3.2 .
We're talking about a single QT_VERSION_CHECK here, protecting a single pair of lines in qtcurve_api.cpp (or defining SH_Menu_SubMenuSloppyCloseTimeout as a macro in an appropriate headerfile).
Yichao, what do you think?
Comment Actions
I'm not familiar with what's the oldest Qt version that is still shipped by supported distributions but if
There are still distributions that don't offer Qt 5.5 and idem on Mac there are OS versions that cannot run anything beyond 5.3.2
And given how simple it is (only the part that actually set the value needs to be ifdefd out, everything else should stay the same with comment showing this is Qt 5.5+ only in the config gui) I agree we should keep supporting 5.3.2.
Comment Actions
Updated as requested
Looks good so far, but how do I get phab to show me the current file versions in the left column, instead of the previous modified versions?
Comment Actions
Ah, indeed.
I must moderate my "looks good so far". The changes are probably as they should bu the patch (or your working copy) needs rebasing before we can test it against git/head.
Comment Actions
The change LGTM. There's another copy in gtk2/common which should ideally be kept in sync (to not make it even harder to merge later) but that shouldn't cause any immediate problems since I don't think that version ever save config file.