> On April 15, 2015, 11:34 p.m., Friedrich W. H. Kossebau wrote: > > plugins/textshape/dialogs/DockerStylesComboModel.cpp, line 192 > > <https://git.reviewboard.kde.org/r/123352/diff/1/?file=361016#file361016line192> > > > > The old code allowed the NoneStyle only as the first item in the source > > model. This code is okay in whatever position it is passed. > > So we trust here the source model to pass the NoneStyle as first, or if > > not, that any other position also makes sense? > > Worth at least a comment, perhaps.
I added a comment that states that the source model already is sorted as wanted. > On April 15, 2015, 11:34 p.m., Friedrich W. H. Kossebau wrote: > > plugins/textshape/dialogs/DockerStylesComboModel.cpp, lines 197-198 > > <https://git.reviewboard.kde.org/r/123352/diff/1/?file=361016#file361016line197> > > > > Why no longer explicitely sort by names? This change is not mentioned > > in the patch description, please tell the motivation for this change. Can > > proper sorting be assumed to exist in the source model? I could not find > > this promise in the API dox of `StylesModel`, so please help me why this > > change is okay :) You are right it is not needed to sort the model as the StylesModel already has the correct order. Should I add a comment to the StylesModel about that? - Thorsten ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123352/#review79010 ----------------------------------------------------------- On April 13, 2015, 2:01 p.m., Thorsten Zachmann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123352/ > ----------------------------------------------------------- > > (Updated April 13, 2015, 2:01 p.m.) > > > Review request for Calligra. > > > Repository: calligra > > > Description > ------- > > This patch fixes the style combo box showing the wrong name for a style after > adding a style. > > To reproduce the problem: > > Create a new stage document (Template Screen Empty): > Add a text shape > Write one line of text > Assign the text the paragraph style Standard. > Change to center alignment. > Use the plus button in the paragraph style to assign it the new name S2. > Change the alignment to right. > Use the plus button in the paragraph style to assign it the new name S1. > > Now open the combobox and scroll to the top. It will show 3 times S1. > The problem is that the index in the StylesModel is changed but not index in > the DockerStylesComboModel.cpp mapping. > Therefor all 3 styles pointing to the same index. > > As the model is recreated all the time it just also newly creates the mapping > simplifying the code in doing so. > It also changes the magic -1 to be a Constant. > > > Diffs > ----- > > plugins/textshape/dialogs/DockerStylesComboModel.cpp 409a9f1 > plugins/textshape/dialogs/StylesModel.h d15651f > plugins/textshape/dialogs/StylesModel.cpp f996824 > > Diff: https://git.reviewboard.kde.org/r/123352/diff/ > > > Testing > ------- > > Tested adding of various style with different names. Worked quite nicly > > > Thanks, > > Thorsten Zachmann > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
