> On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote: > > The following tests are broken - > > > > 1. dialog_positioning.qml > > 2. dialog_positioning2.qml - The animation seems incorrect. Maybe that was > > always different and I did not notice? > > 3. dialog_visualParentChange.qml
1 and 3 were casued by replacing syncToMainItemSize(), it's fixed now 2 is ok: the lack of animation is caused by the window flags, kwin doesn't animate Qt::Popup windows > On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote: > > src/plasmaquick/dialog.cpp, line 470 > > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line470> > > > > Why? It's being done at the end of the function. What was breaking? > > > > Also, considering that updateTheme internally uses the frameSvgItem's > > size, we want to call it after the size has been updated. > > > > Unless I'm missing something? before the first call of updateTheme() there is no svg loaded, so margins will always be 0 > On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote: > > src/plasmaquick/dialog.cpp, line 919 > > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line919> > > > > If you change the location doesn't the position also change? I imagine > > just syncing the borders won't be enough. the default position may change, yes. getting back to calling syncToMainItemSize > On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote: > > src/plasmaquick/dialog.cpp, line 1069 > > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line1069> > > > > Oh. I didn't realize we had cases when the mainItem could be 0. > > > > Could you please add an assert in syncToMainItemSize for checking the > > mainItem. yes, if the qml part doesn't assign it, there will be none, syncToMainItem already checks if it does exists > On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote: > > src/plasmaquick/dialog.cpp, line 946 > > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line946> > > > > Why set this variable at all? It's being changed again at the end of > > the function and not being used anywhere else. slotMainItemSizeChanged won't be executed in this case > On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote: > > src/plasmaquick/dialog.cpp, line 301 > > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line301> > > > > Would this actually do anything? > > > > resizeOrigin = MainItem; > > // do stuff which changes the size and sends a resizeEvent > > resizeOrigin = Undefined; > > > > Then the resizeEvent would be called with the resizeOrigin as > > "Undefined". Hmm, I belive there was going to be an assert. when the width of mainItem is actually changed in this function, causes slotMainItemSizeChanged to be called > On Sept. 17, 2014, 2:54 p.m., Vishesh Handa wrote: > > src/plasmaquick/dialog.cpp, line 771 > > <https://git.reviewboard.kde.org/r/120235/diff/2/?file=312805#file312805line771> > > > > So the visual parent changes, but the position is never updated? Only > > the borders synced? Wouldn't that break changing the visual parent. > > > > I believe I wrote a test for test - dialog_visualParentChange.qml yep, the test was broken, fixed now - Marco ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/120235/#review66744 ----------------------------------------------------------- On Sept. 17, 2014, 2:32 p.m., Marco Martin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/120235/ > ----------------------------------------------------------- > > (Updated Sept. 17, 2014, 2:32 p.m.) > > > Review request for Plasma. > > > Repository: plasma-framework > > > Description > ------- > > A dialog can be resize for two reasons: the mainItem size changes, or the > dialog size changes. > > the first can happen programmatically, caused by the Layout, or just by > assigning the width. > > the second can be caused either programmatically, assigning the size of the > dialog or externally by the windowmanager, that is the only one theat in the > end has the only final control of the window size > > > Diffs > ----- > > src/plasmaquick/dialog.cpp 79a871b > tests/dialog_minWidthHeightRepositioning.qml 37bd622 > > Diff: https://git.reviewboard.kde.org/r/120235/diff/ > > > Testing > ------- > > > Thanks, > > Marco Martin > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
