Am Donnerstag 20 August 2009 04:44:29 schrieb 潘卫平(Peter Pan): > Lukas Appelhans 写道: > > Am Mittwoch 19 August 2009 15:51:18 schrieb 潘卫平(Peter Pan): > >> Lukas Appelhans 写道: > >>> Am Mittwoch 19 August 2009 14:25:09 schrieb 潘卫平(Peter Pan): > >>>> Lukas Appelhans 写道: > >>>>> Am Mittwoch 19 August 2009 05:50:08 schrieb 潘卫平(Peter Pan): > >>>>>> 潘卫平(Peter Pan) 写道: > >>>>>>> Aaron J. Seigo 写道: > >>>>>>>> On Friday 14 August 2009, 潘卫平(Peter Pan) wrote: > >>>>>>>>> svn r 1011382 > >>>>>>>> > >>>>>>>> there are a couple issues with this patch, unfortunately. first, > >>>>>>>> it introduces a modal dialog. that will block the rest of plasma. > >>>>>>>> not good. > >>>>>>>> > >>>>>>>> :/ > >>>>>>> > >>>>>>> That's really not good. > >>>>>>> > >>>>>>>> second, the button names are just "Ok" and "Cancel", they should > >>>>>>>> be changed to having meaningful labels that say _what_ will happen > >>>>>>>> if "Ok" or "Cancel" is pressed. but that's a moot point, because > >>>>>>>> we really can't have a modal dialog here. > >>>>>>>> > >>>>>>>> is there any use case where it makes sense to have more than one > >>>>>>>> icon for the _same_ application or file? i can't think of one. so > >>>>>>>> i'd suggest just silently dropping duplicates. > >>>>>>> > >>>>>>> I prefer to show user a warning message rather than drop it > >>>>>>> silently. > >>>>>>> > >>>>>>>> ------------------------------------------------------------------ > >>>>>>>>-- -- -- > >>>>>>>> > >>>>>>>> _______________________________________________ > >>>>>>>> Plasma-devel mailing list > >>>>>>>> Plasma-devel@kde.org > >>>>>>>> https://mail.kde.org/mailman/listinfo/plasma-devel > >>>>>> > >>>>>> Every time you want to add an application, call checkDuplicateUrls() > >>>>>> first.In this function, I give user a hint when we find duplicate > >>>>>> URLs, then ignore them. > >>>>>> > >>>>>> And setModal(false) for KMessageBox. > >>>>>> > >>>>>> Regards > >>>>> > >>>>> Mmh, I don't like that we iterate through the list 2 times, we should > >>>>> just remove the iteration for checkin duplicates in the addProgram() > >>>>> method imo... > >>>> > >>>> I prefer to make the applications in quicklaunch unique, not allow > >>>> duplicating. Because I don't like that quicklaunch is too wide. > >>> > >>> Yeah, sure, but why do we iterate through the list 2 times? One time to > >>> show the dialog and one time to remove duplicates? that doesn't make > >>> much sense to me... :/ > >> > >> oh, because KDialog->show() will return immediately, so first time, I > >> prepare a message for KMessageBox. > >> If we checkin duplicates in addProgram()'s iteration, only the last > >> duplicate application will be shown. > >> > >> And second time iteration, remove duplicates. > > > > Well why not let checkDuplicates() return a cleared KUrl::List where all > > duplicates are removed? > > I got it. :) This patch looks nice to me... although I think a native english speaker should have a look at the string used in the dialog... but ok, from coding side this patch is ready to go into trunk I think...
Lukas > > > Lukas > > > >>>>> Also the KDialog way seems a bit too much to me, isn't there a way > >>>>> to just get a KMessageBox like the command we got before? > >>>> > >>>> KMessageBox needs a KDialog parameter. > >>>> I can't find another way if we use KMessageBox. > >>> > >>> Okee then leave it that way :) > >>> > >>> Lukas > >>> > >>>>> Lukas > >>>>> _______________________________________________ > >>>>> Plasma-devel mailing list > >>>>> Plasma-devel@kde.org > >>>>> https://mail.kde.org/mailman/listinfo/plasma-devel > >>>> > >>>> Regards > >>> > >>> _______________________________________________ > >>> Plasma-devel mailing list > >>> Plasma-devel@kde.org > >>> https://mail.kde.org/mailman/listinfo/plasma-devel > > > > _______________________________________________ > > Plasma-devel mailing list > > Plasma-devel@kde.org > > https://mail.kde.org/mailman/listinfo/plasma-devel > > Regards _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
