Comment 14 for bug 1013945

Revision history for this message
In , Steve Dodier-Lazaro (sidi) wrote :

Hi,

Thanks for the new patches! I tested on ArchLinux 3.18 64Bits and the exponential slider feels *much* better to use. You're on the way to making a very nice contribution :-) A couple of comments below:

- Somehow I cannot go further than 5h40. Is that expected? I would have expected 6 hours (old GUI was 5h50). Could you please round this to 6?

- Keyboard navigation should always increment by at least one. You could make the keyboard event handler move the slider by several increments until it reaches a higher mapped value, though that would be computationally wasteful. Ideally, computed the next mapped slider value that should be used from the existing value in one calculation

- pow is supported on _BSD_SOURCE || _SVID_SOURCE || _XOPEN_SOURCE >= 600 || _ISOC99_SOURCE || _POSIX_C_SOURCE >= 200112L; @Core Developers, Distributors: is there a system for which using pow would be a problem?

- Could you please create a DEFINE for the min and max values in the code? And then, when storing to xfconf, could you please map the min/"Never" value to 0 instead of what it actually is? And when you load 0 from Xfconf, map it to whichever slider value will show "Never" See below for why

- @XFPM Maintainers: Distributors must be warned that they *must* map existing Xfconf keys valued 14 onto a value of 0 or 4 from now on. This is because 14 means "Never" (it's below the minimum which is 15). However, when shortening the min delay to 4, 14 is then read like a normal valid duration.

In an ideal world, you would provide a patch for:
- moving from 14 to a defined value, and handling the storing 0 in xfconf bit
- replacing the existing 15 minutes by 5 minutes (and also to change the values in the .ui for which you can't rely on a define)
- implementing the logarithmic slider with working keyboard navigation

This way if developers want to delay changing the minimum slider value (because of the 14 minute glitch), they can still apply your other changes.

Thanks