Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 18, 2014, 9:09 a.m.) Status -- This change has been discarded. Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.cpp 2f8227c src/plasmaquick/dialog.h 4009222 Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review60277 --- Marco, any chance for a second run of review on that patch? - Kevin Ottens On June 4, 2014, 10:43 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 10:43 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.cpp 2f8227c src/plasmaquick/dialog.h 4009222 Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 17, 2014, 4:11 p.m., Kevin Ottens wrote: Marco, any chance for a second run of review on that patch? i still not like this patch.. - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review60277 --- On June 4, 2014, 10:43 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 10:43 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.cpp 2f8227c src/plasmaquick/dialog.h 4009222 Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 17, 2014, 4:11 p.m., Kevin Ottens wrote: Marco, any chance for a second run of review on that patch? Marco Martin wrote: i still not like this patch.. Should be dropped then? Or it can get somewhere? Feel free to drop it if you consider it a dead end. I prefer that to open patches lingering on forever. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review60277 --- On June 4, 2014, 10:43 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 10:43 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.cpp 2f8227c src/plasmaquick/dialog.h 4009222 Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 17, 2014, 4:11 p.m., Kevin Ottens wrote: Marco, any chance for a second run of review on that patch? Marco Martin wrote: i still not like this patch.. Kevin Ottens wrote: Should be dropped then? Or it can get somewhere? Feel free to drop it if you consider it a dead end. I prefer that to open patches lingering on forever. I don't seem to be able to do so. are permissions for doing so to be asked somewhere? - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review60277 --- On June 4, 2014, 10:43 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 10:43 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.cpp 2f8227c src/plasmaquick/dialog.h 4009222 Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 17, 2014, 4:11 p.m., Kevin Ottens wrote: Marco, any chance for a second run of review on that patch? Marco Martin wrote: i still not like this patch.. Kevin Ottens wrote: Should be dropped then? Or it can get somewhere? Feel free to drop it if you consider it a dead end. I prefer that to open patches lingering on forever. Marco Martin wrote: I don't seem to be able to do so. are permissions for doing so to be asked somewhere? I thought you could. :-) I can close it for you if you want, David E. should be able to as well. Should we discard it? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review60277 --- On June 4, 2014, 10:43 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 10:43 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.cpp 2f8227c src/plasmaquick/dialog.h 4009222 Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 4, 2014, 7:30 p.m., Marco Martin wrote: what is a valid use case where qml needs to know where the dialog actually is?(not hypothetical please) doing the animation on the proper direction can be tracked completely internally also, plasmoids should never ever do a screen edge animation when in the middle of the desktop, the animation looks bad where it's clipped if it's not against a panel or a screen edge David Edmundson wrote: Milou places the most relevant item closest to the launcher. Marco Martin wrote: Hmm, it would be relevant only in the case milou is iconified and in the desktop, that seems quite a narrow case. api wise it may make sense a Plasma::Types::Direction visualParentDirection, *but* it would have to be exported in plasmoid as well, and I don't want anything like that here. API that is added ad hoc for a single, extremely narrow use case is never a good idea. David Edmundson wrote: kickoff changes direction too so the toolbar is closest to the launcher. However, I'm not too fussed about the new property, I can remove the Q_PROPERTY and add it if we decide it's needed. The important part for me is making it show the correct borders and animate in the right direction. I don't think animating or borders is a problem, because the location dependent borders should be disabled only when at screen edges, when the location is the real one, and the animation as well should be done only when at screen edge or in panel as well. If the property ends up being really needed, i can see it working as a direction (instead of location enum), without any setter. To be usable in applets however, it would still need a plasmoid property, like popupsDirection, wouldn't even be that bad, but for it to work it would need to track the scene position of the applet, thing that seems possible in 5.3 but heavy on cpu - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review59234 --- On June 4, 2014, 10:43 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 10:43 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.cpp 2f8227c src/plasmaquick/dialog.h 4009222 Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 118538: Add a property containing the real edge a dialog is shown on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review59234 --- what is a valid use case where qml needs to know where the dialog actually is?(not hypothetical please) doing the animation on the proper direction can be tracked completely internally also, plasmoids should never ever do a screen edge animation when in the middle of the desktop, the animation looks bad where it's clipped if it's not against a panel or a screen edge - Marco Martin On June 4, 2014, 7:07 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 7:07 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 4, 2014, 7:30 p.m., Marco Martin wrote: what is a valid use case where qml needs to know where the dialog actually is?(not hypothetical please) doing the animation on the proper direction can be tracked completely internally also, plasmoids should never ever do a screen edge animation when in the middle of the desktop, the animation looks bad where it's clipped if it's not against a panel or a screen edge Milou places the most relevant item closest to the launcher. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review59234 --- On June 4, 2014, 7:07 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 7:07 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 4, 2014, 7:30 p.m., Marco Martin wrote: what is a valid use case where qml needs to know where the dialog actually is?(not hypothetical please) doing the animation on the proper direction can be tracked completely internally also, plasmoids should never ever do a screen edge animation when in the middle of the desktop, the animation looks bad where it's clipped if it's not against a panel or a screen edge David Edmundson wrote: Milou places the most relevant item closest to the launcher. Hmm, it would be relevant only in the case milou is iconified and in the desktop, that seems quite a narrow case. api wise it may make sense a Plasma::Types::Direction visualParentDirection, *but* it would have to be exported in plasmoid as well, and I don't want anything like that here. API that is added ad hoc for a single, extremely narrow use case is never a good idea. - Marco --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review59234 --- On June 4, 2014, 7:07 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 7:07 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
On June 4, 2014, 7:30 p.m., Marco Martin wrote: what is a valid use case where qml needs to know where the dialog actually is?(not hypothetical please) doing the animation on the proper direction can be tracked completely internally also, plasmoids should never ever do a screen edge animation when in the middle of the desktop, the animation looks bad where it's clipped if it's not against a panel or a screen edge David Edmundson wrote: Milou places the most relevant item closest to the launcher. Marco Martin wrote: Hmm, it would be relevant only in the case milou is iconified and in the desktop, that seems quite a narrow case. api wise it may make sense a Plasma::Types::Direction visualParentDirection, *but* it would have to be exported in plasmoid as well, and I don't want anything like that here. API that is added ad hoc for a single, extremely narrow use case is never a good idea. kickoff changes direction too so the toolbar is closest to the launcher. However, I'm not too fussed about the new property, I can remove the Q_PROPERTY and add it if we decide it's needed. The important part for me is making it show the correct borders and animate in the right direction. - David --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/#review59234 --- On June 4, 2014, 7:07 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 7:07 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs - src/plasmaquick/dialog.h 4009222 src/plasmaquick/dialog.cpp 2f8227c Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 118538: Add a property containing the real edge a dialog is shown on
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/118538/ --- (Updated June 4, 2014, 10:43 p.m.) Review request for KDE Frameworks and Plasma. Repository: plasma-framework Description --- Add a property containing the real edge a dialog is shown on. If a dialog is meant to be shown on the left of an item, if there is not enough space it will be shown on the right. We need to keep track of where we're actually shown in order to show the correct borders and perform the animation in the correct direction. It is exposed as a property in case the dialog itself needs to know which side of the parent item it is, for example if needs to show an arrow to the parent. -- Fixed some serious copy + paste fails in the hitting the bottom edge of the screen check. Diffs (updated) - src/plasmaquick/dialog.cpp 2f8227c src/plasmaquick/dialog.h 4009222 Diff: https://git.reviewboard.kde.org/r/118538/diff/ Testing --- Ran dialog_positioning.qml in all 8 combinations of edges + preferred locations. (i.e placed near left edge, showed item with location set to both left and right) Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
