Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
On Oct. 2, 2014, 4:04 p.m., Kai Uwe Broulik wrote: applets/digital-clock/package/contents/ui/DigitalClock.qml, line 231 https://git.reviewboard.kde.org/r/120461/diff/1/?file=316057#file316057line231 Is this needed? Martin Klapetek wrote: For tooltip? Otherwise I don't think it is Removed it, has no use. - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/#review67812 --- On Oct. 2, 2014, 5:05 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Oct. 2, 2014, 5:05 p.m.) Review request for Plasma. Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Oct. 3, 2014, 9:41 a.m.) Status -- This change has been marked as submitted. Review request for Plasma. Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Oct. 2, 2014, 3:27 p.m.) Review request for Plasma. Changes --- Add screenshots Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments (updated) Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/#review67810 --- Ship it! I tried it a bit, it seems to work fine - Marco Martin On Oct. 2, 2014, 1:27 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Oct. 2, 2014, 1:27 p.m.) Review request for Plasma. Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/#review67811 --- Ship it! Nice work! applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120461/#comment47214 There may be a corner case here, when the formfactor changes to Planar or MediaCenter (i.e. other), Layout.* properties may still be set. I don't know if this is much of an issue in practise, as that kind of formfactor change should happen very rarely, but might be something to improve (in the future). - Sebastian Kügler On Oct. 2, 2014, 1:27 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Oct. 2, 2014, 1:27 p.m.) Review request for Plasma. Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/#review67812 --- File Attachment: Variants in vertical panel - digital-clock2.png https://git.reviewboard.kde.org//r/120461/#fcomment270 Is it the panel which makes it not look horizontally centered? applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120461/#comment47215 This could be compacted to one-liners without braces applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120461/#comment47216 Could this be based on some unit? applets/digital-clock/package/contents/ui/DigitalClock.qml https://git.reviewboard.kde.org/r/120461/#comment47217 Is this needed? - Kai Uwe Broulik On Okt. 2, 2014, 1:27 nachm., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Okt. 2, 2014, 1:27 nachm.) Review request for Plasma. Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/#review67813 --- Ship it! Ship It! - Kai Uwe Broulik On Okt. 2, 2014, 1:27 nachm., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Okt. 2, 2014, 1:27 nachm.) Review request for Plasma. Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
On Oct. 2, 2014, 3:57 p.m., Sebastian Kügler wrote: applets/digital-clock/package/contents/ui/DigitalClock.qml, line 216 https://git.reviewboard.kde.org/r/120461/diff/1/?file=316057#file316057line216 There may be a corner case here, when the formfactor changes to Planar or MediaCenter (i.e. other), Layout.* properties may still be set. I don't know if this is much of an issue in practise, as that kind of formfactor change should happen very rarely, but might be something to improve (in the future). Well now that you mentioned it, I never tried it on desktop. This is not enough for desktop/planar, so...fix incoming ^_- - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/#review67811 --- On Oct. 2, 2014, 3:27 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Oct. 2, 2014, 3:27 p.m.) Review request for Plasma. Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
On Oct. 2, 2014, 4:04 p.m., Kai Uwe Broulik wrote: File Attachment: Variants in vertical panel - digital-clock2.png https://git.reviewboard.kde.org/r/120461/#fcomment271 Is it the panel which makes it not look horizontally centered? I think so, the vertical variant has Layout.fillWidth: true but when you paint rectangles, you can see a couple pixels offset at the side. There are no margins being set. It's possibly the blue strip. On Oct. 2, 2014, 4:04 p.m., Kai Uwe Broulik wrote: applets/digital-clock/package/contents/ui/DigitalClock.qml, lines 219-220 https://git.reviewboard.kde.org/r/120461/diff/1/?file=316057#file316057line219 Could this be based on some unit? I removed it completely, replaced with minSize based on units On Oct. 2, 2014, 4:04 p.m., Kai Uwe Broulik wrote: applets/digital-clock/package/contents/ui/DigitalClock.qml, line 231 https://git.reviewboard.kde.org/r/120461/diff/1/?file=316057#file316057line231 Is this needed? For tooltip? Otherwise I don't think it is - Martin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/#review67812 --- On Oct. 2, 2014, 3:27 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Oct. 2, 2014, 3:27 p.m.) Review request for Plasma. Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request 120461: Refactor DigitalClock applet to use multiple labels and states
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120461/ --- (Updated Oct. 2, 2014, 5:05 p.m.) Review request for Plasma. Changes --- Fixed desktop/planar mode, couple other minor issues Repository: plasma-workspace Description --- Right now the clock applet is using one single label (besides the sizehelper) to fit time, timezone and also date. This has several drawbacks like for example the whole label must use one font size, one eliding etc. Putting everything into its own label gives way more flexibility as we can position and size things independently - for example making the time bigger font than the timezone or date and elide long timezone name without omitting the date. The sizing is now also simpler and more robust. This patch adds the time and timezone labels into Flow which changes direction based on the layout (in vertical panels layouts things vertically, horizontal panels either vertically or horizontally depending on date being shown or not), the date label is then always appended at the bottom. The reason for dateLabel not being in the Flow is that in horizontal panels we want the date label always go to the bottom and be center aligned and Flow does not support horizontal alignment of its children. Two anchors seems much easier in this case. This removes two i18n strings and works around having to add another one (line 352 in the patch) - it's not kosher but I did it so it can still be merged for 5.1 which I would be strongly in favor of. If you decide it should wait for 5.2, I'll add the i18n at the line 352). Finally, this adds states for separate handling of vertical and horizontal panels, which has cleaned the code quite a lot from all the vertical ? complex_a_stuff : complex_b_stuff. Sorry it took so long, I kept quickly adding more things to finish it asap until the point where I had to stop and start from scratch. Result is much simpler much cleaner code. Diffs (updated) - applets/digital-clock/package/contents/ui/DigitalClock.qml 4853716 Diff: https://git.reviewboard.kde.org/r/120461/diff/ Testing --- I did try a thorough testing of all the features in panels on all screen edges, all seems good. File Attachments Variants in horizontal panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/30deb86a-4ea6-45b7-b2ed-cfbab0c76e68__digital-clock1.png Variants in vertical panel https://git.reviewboard.kde.org/media/uploaded/files/2014/10/02/4fd5dca9-2e90-419e-a60d-f998b8b2bd7e__digital-clock2.png Thanks, Martin Klapetek ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
