Re: Review Request: Custom combo boxes for Plasma::ComboBox
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/ --- (Updated 2009-08-30 16:39:32.923467) Review request for Plasma, Aaron Seigo and Marco Martin. Changes --- New approach, constructor replaced by setNativeWidget() method. It's time to decide which widgets should also have this method, personally I think that at least LineEdit, SpinBox, TextEdit should have it (others usually are not sub classed, at least as far as I know). Summary (updated) --- This patch adds possibility to use custom combo box widgets (sub classed from KComboBox) instead of plain KComboBox. Diffs (updated) - /trunk/KDE/kdelibs/plasma/widgets/combobox.h 1016281 /trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 1016281 Diff: http://reviewboard.kde.org/r/1094/diff Testing (updated) --- Compiles and should work, but not tested. Screenshots --- Plasma themed KHistoryComboBox in Run Command applet http://reviewboard.kde.org/r/1094/s/151/ Thanks, Michal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Custom combo boxes for Plasma::ComboBox
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/#review2193 --- Ship it! i'm fine with this going in; line edit and text edits too, i think. but spinbox? i don't think it's that common to subclass it ...? - Aaron On 2009-08-30 16:39:32, Michal Dutkiewicz wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/ --- (Updated 2009-08-30 16:39:32) Review request for Plasma, Aaron Seigo and Marco Martin. Summary --- This patch adds possibility to use custom combo box widgets (sub classed from KComboBox) instead of plain KComboBox. Diffs - /trunk/KDE/kdelibs/plasma/widgets/combobox.h 1016281 /trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 1016281 Diff: http://reviewboard.kde.org/r/1094/diff Testing --- Compiles and should work, but not tested. Screenshots --- Plasma themed KHistoryComboBox in Run Command applet http://reviewboard.kde.org/r/1094/s/151/ Thanks, Michal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Custom combo boxes for Plasma::ComboBox
On 2009-08-30 19:40:51, Aaron Seigo wrote: i'm fine with this going in; line edit and text edits too, i think. but spinbox? i don't think it's that common to subclass it ...? OK, so maybe I'll add these widgets and then upload one bigger patch, tomorrow I'll have someone to test code. In case if it will be needed also for others widgets then it could be added later, there is plenty of time before 4.4. ;-) - Michal --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/#review2193 --- On 2009-08-30 16:39:32, Michal Dutkiewicz wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/ --- (Updated 2009-08-30 16:39:32) Review request for Plasma, Aaron Seigo and Marco Martin. Summary --- This patch adds possibility to use custom combo box widgets (sub classed from KComboBox) instead of plain KComboBox. Diffs - /trunk/KDE/kdelibs/plasma/widgets/combobox.h 1016281 /trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 1016281 Diff: http://reviewboard.kde.org/r/1094/diff Testing --- Compiles and should work, but not tested. Screenshots --- Plasma themed KHistoryComboBox in Run Command applet http://reviewboard.kde.org/r/1094/s/151/ Thanks, Michal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Custom combo boxes for Plasma::ComboBox
On 2009-07-23 04:12:32, Aaron Seigo wrote: if this is needed (which it may be in this case) then it should not be done using a constructor imo but rather a setNativeWidget method. that a combobox is created and then discarded in the process is a non-issue (it's not a hot path and the execution time of that code is quite negligable). Right, I was thinking also about this solution. Maybe we should move all code that sets style and connects signals and slots to that method and call it with new widget from constructor (to avoid code duplication)? I'll prepare new patch later. - Michal --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/#review1722 --- On 2009-07-21 15:55:33, Michal Dutkiewicz wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/ --- (Updated 2009-07-21 15:55:33) Review request for Plasma and Marco Martin. Summary --- This patch adds possibility to use custom combo box widgets (subclassed from KComboBox) instead of plain KComboBox. It's intended for 4.3 (better later than never..., didn't know that there is still no possibility to do something like this and no possibility to create custom Plasma themed widget). It adds new constructor to set widget without creating new like in first. It will be used by Run Command applet (KHistoryComboBox, waiting for theming possibility for very long time) and probably by others in the future. If it will be accepted I'll probably prepare similar patches to some other widgets (most common) for consistency and bigger flexibility that comes without costs (as far as I know). Diffs - /trunk/KDE/kdelibs/plasma/widgets/combobox.h 1000582 /trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 1000582 Diff: http://reviewboard.kde.org/r/1094/diff Testing --- Tested by my friend, compiles and works. Screenshots --- Plasma themed KHistoryComboBox in Run Command applet http://reviewboard.kde.org/r/1094/s/151/ Thanks, Michal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Custom combo boxes for Plasma::ComboBox
On Thursday 23 July 2009, Michal Dutkiewicz wrote: On 2009-07-23 04:12:32, Aaron Seigo wrote: if this is needed (which it may be in this case) then it should not be done using a constructor imo but rather a setNativeWidget method. that a combobox is created and then discarded in the process is a non-issue (it's not a hot path and the execution time of that code is quite negligable). Right, I was thinking also about this solution. Maybe we should move all code that sets style and connects signals and slots to that method and call it with new widget from constructor (to avoid code duplication)? +1 so the constructor will just call this I'll prepare new patch later. - Michal --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/#review1722 --- On 2009-07-21 15:55:33, Michal Dutkiewicz wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/ --- (Updated 2009-07-21 15:55:33) Review request for Plasma and Marco Martin. Summary --- This patch adds possibility to use custom combo box widgets (subclassed from KComboBox) instead of plain KComboBox. It's intended for 4.3 (better later than never..., didn't know that there is still no possibility to do something like this and no possibility to create custom Plasma themed widget). It adds new constructor to set widget without creating new like in first. It will be used by Run Command applet (KHistoryComboBox, waiting for theming possibility for very long time) and probably by others in the future. If it will be accepted I'll probably prepare similar patches to some other widgets (most common) for consistency and bigger flexibility that comes without costs (as far as I know). Diffs - /trunk/KDE/kdelibs/plasma/widgets/combobox.h 1000582 /trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 1000582 Diff: http://reviewboard.kde.org/r/1094/diff Testing --- Tested by my friend, compiles and works. Screenshots --- Plasma themed KHistoryComboBox in Run Command applet http://reviewboard.kde.org/r/1094/s/151/ Thanks, Michal -- Marco Martin ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Custom combo boxes for Plasma::ComboBox
On Tuesday 21 July 2009 17:55:33 Michal Dutkiewicz wrote: It's intended for 4.3 (better later than never..., didn't know that there is still no possibility to do something like this and no possibility to create custom Plasma themed widget). Note that it won't be accepted in 4.3, it's a feature addition and 4.3 is closed for months for new features. Next stop is 4.4. -- sebas http://www.kde.org | http://vizZzion.org | GPG Key ID: 9119 0EF9 signature.asc Description: This is a digitally signed message part. ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Custom combo boxes for Plasma::ComboBox
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/#review1722 --- if this is needed (which it may be in this case) then it should not be done using a constructor imo but rather a setNativeWidget method. that a combobox is created and then discarded in the process is a non-issue (it's not a hot path and the execution time of that code is quite negligable). - Aaron On 2009-07-21 15:55:33, Michal Dutkiewicz wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/ --- (Updated 2009-07-21 15:55:33) Review request for Plasma and Marco Martin. Summary --- This patch adds possibility to use custom combo box widgets (subclassed from KComboBox) instead of plain KComboBox. It's intended for 4.3 (better later than never..., didn't know that there is still no possibility to do something like this and no possibility to create custom Plasma themed widget). It adds new constructor to set widget without creating new like in first. It will be used by Run Command applet (KHistoryComboBox, waiting for theming possibility for very long time) and probably by others in the future. If it will be accepted I'll probably prepare similar patches to some other widgets (most common) for consistency and bigger flexibility that comes without costs (as far as I know). Diffs - /trunk/KDE/kdelibs/plasma/widgets/combobox.h 1000582 /trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 1000582 Diff: http://reviewboard.kde.org/r/1094/diff Testing --- Tested by my friend, compiles and works. Screenshots --- Plasma themed KHistoryComboBox in Run Command applet http://reviewboard.kde.org/r/1094/s/151/ Thanks, Michal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Custom combo boxes for Plasma::ComboBox
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/#review1705 --- I like the idea,since doesn't make sense a new plasma widget for every conceivale thing, i think adding the possibility (just for c++ ones?) of putting a subclass in does make the thing more flexible while not putting too many exported symbols of course if it's ok for the combobox it should be done for all proxywidgets, either all of them or nothing :) - Marco On 2009-07-21 15:55:33, Michal Dutkiewicz wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/ --- (Updated 2009-07-21 15:55:33) Review request for Plasma and Marco Martin. Summary --- This patch adds possibility to use custom combo box widgets (subclassed from KComboBox) instead of plain KComboBox. It's intended for 4.3 (better later than never..., didn't know that there is still no possibility to do something like this and no possibility to create custom Plasma themed widget). It adds new constructor to set widget without creating new like in first. It will be used by Run Command applet (KHistoryComboBox, waiting for theming possibility for very long time) and probably by others in the future. If it will be accepted I'll probably prepare similar patches to some other widgets (most common) for consistency and bigger flexibility that comes without costs (as far as I know). Diffs - /trunk/KDE/kdelibs/plasma/widgets/combobox.h 1000582 /trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 1000582 Diff: http://reviewboard.kde.org/r/1094/diff Testing --- Tested by my friend, compiles and works. Screenshots --- Plasma themed KHistoryComboBox in Run Command applet http://reviewboard.kde.org/r/1094/s/151/ Thanks, Michal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Custom combo boxes for Plasma::ComboBox
On 2009-07-21 18:37:25, Marco Martin wrote: I like the idea,since doesn't make sense a new plasma widget for every conceivale thing, i think adding the possibility (just for c++ ones?) of putting a subclass in does make the thing more flexible while not putting too many exported symbols of course if it's ok for the combobox it should be done for all proxywidgets, either all of them or nothing :) As discussed on IRC, for now we don't need that (could be done with ugly way be setting new widget for proxy that has style taken from widget created in Plasma widget) and there is possible problem with widgets created using: Plasma::ComboBox(NULL)... But we could try to find better solution for 4.4 probably, by adding method that sets own widget instead current, connects signals and slots and sets style. Maybe someone has better idea? - Michal --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/#review1705 --- On 2009-07-21 15:55:33, Michal Dutkiewicz wrote: --- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/1094/ --- (Updated 2009-07-21 15:55:33) Review request for Plasma and Marco Martin. Summary --- This patch adds possibility to use custom combo box widgets (subclassed from KComboBox) instead of plain KComboBox. It's intended for 4.3 (better later than never..., didn't know that there is still no possibility to do something like this and no possibility to create custom Plasma themed widget). It adds new constructor to set widget without creating new like in first. It will be used by Run Command applet (KHistoryComboBox, waiting for theming possibility for very long time) and probably by others in the future. If it will be accepted I'll probably prepare similar patches to some other widgets (most common) for consistency and bigger flexibility that comes without costs (as far as I know). Diffs - /trunk/KDE/kdelibs/plasma/widgets/combobox.h 1000582 /trunk/KDE/kdelibs/plasma/widgets/combobox.cpp 1000582 Diff: http://reviewboard.kde.org/r/1094/diff Testing --- Tested by my friend, compiles and works. Screenshots --- Plasma themed KHistoryComboBox in Run Command applet http://reviewboard.kde.org/r/1094/s/151/ Thanks, Michal ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
