On March 30, 2015, 5:48 p.m., Martin Klapetek wrote: > > If the host application is required to check for a property, then this > > should be documented as well. > > > > I'm unsure about the use-case there though. Why do we need to filter the > > actions? If we just want chat, wouldn't it be better to directly request it > > to KTp? > > Eike Hein wrote: > > I'm unsure about the use-case there though. Why do we need to filter > the actions? If we just want chat, wouldn't it be better to directly request > it to KTp? > > We don't want frontends to be tied to KTp in the long run - we want other > messengers (e.g. Konversation) to integrate with KPeople, and a programatic > way to start a conversation using the user's preferred messenger (we already > have a Default Apps setting for one), depending on presence. > > Something like startTextChat(PersonData *data) would be much better to > hide the implementation instead of relying on action list sorting to return > the preferred TextChatAction first, but as KTp is the only available backend > right now this is fine for use in Plasma 5.3, there's no need to rush the API > design. > > There was some email forth and back on this you were CC'd on btw. > > Martin Klapetek wrote: > Clients are not required for checking the property, they can if they > want/need to, as Eike explains. Adding more metadata to the actions only > allows for better flexibility, this would also allow for example to group > related actions together in say address book. > > Aleix Pol Gonzalez wrote: > Still, the property is not documented. > > We don't need to rush an API design, but we definitely need to find ways > to move away from the current QWidgets-based approach. > > Anyway, I guess this works, I'm happy to have it in and see how it > develops. > > Martin Klapetek wrote: > Also I've put it with PersonData because the plugins implementing the > actions do not include actions.h but they always include persondata.h because > it's being passed to it. I don't think that putting it into actions.h makes > too much sense because that's client only, not for plugins which also need > that enum (and the plugins don't link to KPeopleWidgets either). But if you > think it should go there, I'll move it there.
I prefer orienting these things for clients than plugins, as it explains better what it's about. Also, if we ever decide to come up with a new API, we'll still have an enum only targetting Actions. Please move to actions.h and explain how to fetch the flag from the QAction instance. - Aleix ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123184/#review78223 ----------------------------------------------------------- On March 30, 2015, 5:03 p.m., Martin Klapetek wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123184/ > ----------------------------------------------------------- > > (Updated March 30, 2015, 5:03 p.m.) > > > Review request for Telepathy and Eike Hein. > > > Repository: kpeople > > > Description > ------- > > Some clients need/want to filter actions based on types, this adds the > actions enum with currently known actions. Plugins returning QList<QAction*> > should then use this enum to set the appropriate type on the action (via > setProperty). > > I've put it to PersonData because all code dealing with actions currently > need to include PersonData header and I'm not sure if it makes sense anywhere > else. > > > Diffs > ----- > > src/persondata.h c3f99a9 > src/widgets/actions.h 2931ef8 > > Diff: https://git.reviewboard.kde.org/r/123184/diff/ > > > Testing > ------- > > > Thanks, > > Martin Klapetek > >
_______________________________________________ KDE-Telepathy mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-telepathy
