Re: Review Request: Provide extra options for date keyword display in KDateComboBox
On Nov. 20, 2011, 10:07 p.m., Albert Astals Cid wrote: This patch modifies the behaviour, maybe it is better if you change KDateComboBox::NoneKeyword to KDateComboBox::NoNoneKeyword And adapt the if accordingly? This way there is no behaviour change at all and makes old users still work. This class was rushed in at the last moment for KDE 4.7, and I doubt whether this particular behaviour is really right, or that many applications will want it. So although it is a minor change in behaviour, I would see it as rectifying a mistake. The function to select a blank date is already there via the clear button in the edit field anyway, so the change only removes the duplication of a feature. - David --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103103/#review8351 --- On Nov. 10, 2011, 12:36 a.m., David Jarvie wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103103/ --- (Updated Nov. 10, 2011, 12:36 a.m.) Review request for kdelibs and John Layt. Description --- KDateComboBox provides the option to display a list of date keywords with the date picker popup. These keywords currently show dates in the past, present and future, together with a no date item. The no date item is a particular problem since not only is it redundant - since there is already a button in the line edit to clear the date value - but I suspect that many applications will not accept a blank date as valid input. This patch adds two new enum values to allow applications to select which of these date keywords are displayed: one enum value shows the no date item which is now not shown by default, and another enum value hides dates in the past. Currently in KAlarm, which I maintain, the display of date keywords is disabled because dates in the past and blank dates are not valid values to enter, and it would be confusing to users to offer them as options. This patch will make it possible to display only present and future date keywords, which will enable it to make use of date keywords. Diffs - kdeui/widgets/kdatecombobox.h 63bf52f kdeui/widgets/kdatecombobox.cpp fc239bc Diff: http://git.reviewboard.kde.org/r/103103/diff/diff Testing --- Tested in KAlarm. Thanks, David Jarvie
Re: Review Request: Provide extra options for date keyword display in KDateComboBox
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103103/#review8351 --- This patch modifies the behaviour, maybe it is better if you change KDateComboBox::NoneKeyword to KDateComboBox::NoNoneKeyword And adapt the if accordingly? This way there is no behaviour change at all and makes old users still work. - Albert Astals Cid On Nov. 10, 2011, 12:36 a.m., David Jarvie wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103103/ --- (Updated Nov. 10, 2011, 12:36 a.m.) Review request for kdelibs and John Layt. Description --- KDateComboBox provides the option to display a list of date keywords with the date picker popup. These keywords currently show dates in the past, present and future, together with a no date item. The no date item is a particular problem since not only is it redundant - since there is already a button in the line edit to clear the date value - but I suspect that many applications will not accept a blank date as valid input. This patch adds two new enum values to allow applications to select which of these date keywords are displayed: one enum value shows the no date item which is now not shown by default, and another enum value hides dates in the past. Currently in KAlarm, which I maintain, the display of date keywords is disabled because dates in the past and blank dates are not valid values to enter, and it would be confusing to users to offer them as options. This patch will make it possible to display only present and future date keywords, which will enable it to make use of date keywords. Diffs - kdeui/widgets/kdatecombobox.h 63bf52f kdeui/widgets/kdatecombobox.cpp fc239bc Diff: http://git.reviewboard.kde.org/r/103103/diff/diff Testing --- Tested in KAlarm. Thanks, David Jarvie
Re: Review Request: Provide extra options for date keyword display in KDateComboBox
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103103/#review8267 --- From a pure perspective of the code it is a +1 from me. I would consider this an API change, and are currently in soft API freeze mode so if you do commit, then please also CC kde-bindings. But... in a way this violates the bug-fixes only policy in kdelibs which is why I'm not clicking the Ship It button. Mainly because I'm not 100% in favor of that policy nor do I fully understand it. Someone else who fully understands the policy needs to comment. - Allen Winter On Nov. 10, 2011, 12:36 a.m., David Jarvie wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103103/ --- (Updated Nov. 10, 2011, 12:36 a.m.) Review request for kdelibs and John Layt. Description --- KDateComboBox provides the option to display a list of date keywords with the date picker popup. These keywords currently show dates in the past, present and future, together with a no date item. The no date item is a particular problem since not only is it redundant - since there is already a button in the line edit to clear the date value - but I suspect that many applications will not accept a blank date as valid input. This patch adds two new enum values to allow applications to select which of these date keywords are displayed: one enum value shows the no date item which is now not shown by default, and another enum value hides dates in the past. Currently in KAlarm, which I maintain, the display of date keywords is disabled because dates in the past and blank dates are not valid values to enter, and it would be confusing to users to offer them as options. This patch will make it possible to display only present and future date keywords, which will enable it to make use of date keywords. Diffs - kdeui/widgets/kdatecombobox.h 63bf52f kdeui/widgets/kdatecombobox.cpp fc239bc Diff: http://git.reviewboard.kde.org/r/103103/diff/diff Testing --- Tested in KAlarm. Thanks, David Jarvie
Review Request: Provide extra options for date keyword display in KDateComboBox
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/103103/ --- Review request for kdelibs and John Layt. Description --- KDateComboBox provides the option to display a list of date keywords with the date picker popup. These keywords currently show dates in the past, present and future, together with a no date item. The no date item is a particular problem since not only is it redundant - since there is already a button in the line edit to clear the date value - but I suspect that many applications will not accept a blank date as valid input. This patch adds two new enum values to allow applications to select which of these date keywords are displayed: one enum value shows the no date item which is now not shown by default, and another enum value hides dates in the past. Currently in KAlarm, which I maintain, the display of date keywords is disabled because dates in the past and blank dates are not valid values to enter, and it would be confusing to users to offer them as options. This patch will make it possible to display only present and future date keywords, which will enable it to make use of date keywords. Diffs - kdeui/widgets/kdatecombobox.h 63bf52f kdeui/widgets/kdatecombobox.cpp fc239bc Diff: http://git.reviewboard.kde.org/r/103103/diff/diff Testing --- Tested in KAlarm. Thanks, David Jarvie
