[PATCH] BUG 203716 give user a hint when adding same application into quicklaunch

[Lukas Appelhans]

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 at 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 at kde.org
> >>>>> https://mail.kde.org/mailman/listinfo/plasma-devel
> >>>>
> >>>> Regards
> >>>
> >>> _______________________________________________
> >>> Plasma-devel mailing list
> >>> Plasma-devel at kde.org
> >>> https://mail.kde.org/mailman/listinfo/plasma-devel
> >
> > _______________________________________________
> > Plasma-devel mailing list
> > Plasma-devel at kde.org
> > https://mail.kde.org/mailman/listinfo/plasma-devel
>
> Regards