[Kde-pim] Review Request 125331: Port part of KCalUtils::IncidenceFormatter to Grantlee template system
Sandro Knauß
sknauss at kde.org
Mon Oct 5 18:03:28 BST 2015
> On Okt. 5, 2015, 3:39 nachm., Sandro Knauß wrote:
> > src/incidenceformatter.cpp, line 337
> > <https://git.reviewboard.kde.org/r/125331/diff/6/?file=409229#file409229line337>
> >
> > for kolab we also need delegator/delegatee for persons. This is done by adding a indetity of that other person -> so that not everytime you find a identity of a person means that it is you (as person). And you may want to mail the delegatee.
> >
> > I would recommend to add the mail in anycase ( if available) and push the decision, if you want to display the mailto url further up the stack.
> >
> > That makes it much more easier to implement delegator/delegatee support properly.
>
> Daniel Vrátil wrote:
> That's for ITIP though, not for events/todos, right?
>
> Additionally, "thatIsMe" is always correct, you never want to mail yourself, right? The email address is resolved against KIdentityManagement::thatIsMe(), so if the even is delegated, the email address is different and we display it correctly.
>
> This only really supresses a clickable email if the email address matches any of your KMail identities.
for having full control over a delegated calendar you need also have an identiy for this delegated folder.
ex: john doe and should handle the calendar from Jane
* john creates a identy for jane, because he needs to act as Jane (send mails with jane as sender)
* with that thatIsMe would also be true for Janes emailaddress ( because a identiy was added)
* and for sure John wants to able to send Jane mails :)
for ITIP we expicitly ask the use, as whom we want to act. That's what I mean with we should not deside these kinds too deeply.
> On Okt. 5, 2015, 3:39 nachm., Sandro Knauß wrote:
> > src/grantlee_plugin/icon.cpp, line 116
> > <https://git.reviewboard.kde.org/r/125331/diff/6/?file=409222#file409222line116>
> >
> > add a "alt" propery for screenreader or people who can't look pictures.
>
> Daniel Vrátil wrote:
> Yeah, but what should the alt say? We only know the filename, nothing about the content...
either make it possible to add a alt text in grantlee or use the iconName. The iconname is better than nothing and mostly it also gives a good idea what the icon do.
- Sandro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125331/#review86382
-----------------------------------------------------------
On Okt. 1, 2015, 8:21 vorm., Daniel Vrátil wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125331/
> -----------------------------------------------------------
>
> (Updated Okt. 1, 2015, 8:21 vorm.)
>
>
> Review request for KDEPIM, Laurent Montel and Volker Krause.
>
>
> Repository: kcalutils
>
>
> Description
> -------
>
> I needed to extend the HTML generated for todos so that it includes some more fields useful for the Phabricator resource. After digging around the code a bit a decided to see if I can port it to Grantlee, which is way easier to maintain and modify and also provides much cleaner separation between logic and representation and this is the result of it :)
>
> This patch does not port the entire IndicendeFormatter to Grantlee, only events, tasks, journals and free/busy, I haven't touched the ITIP formatter yet.
>
> The new code works just fine, but I consider it more of a proof-of-concept than a ready-to-ship patch and I'd like to get your input regarding the use of Grantlee - should we use a QObject wrapper for KCalCore::Incidence instead of QVariantHash, or maybe use the Grantlee MetaType system directly for thinner-than-QObject wrappers, should we use the custom Grantlee plugin, ...?
>
> Also please see the mail on kde-pim regarding where should we put the plugin :)
>
>
> Diffs
> -----
>
> CMakeLists.txt d145b6a
> Messages.sh PRE-CREATION
> autotests/CMakeLists.txt a7ce2f1
> autotests/data/event-1.html PRE-CREATION
> autotests/data/event-1.ical PRE-CREATION
> autotests/data/event-2.html PRE-CREATION
> autotests/data/event-2.ical PRE-CREATION
> autotests/data/freebusy-1.html PRE-CREATION
> autotests/data/freebusy-1.ical PRE-CREATION
> autotests/data/journal-1.html PRE-CREATION
> autotests/data/journal-1.ical PRE-CREATION
> autotests/data/todo-1.html PRE-CREATION
> autotests/data/todo-1.ical PRE-CREATION
> autotests/testincidenceformatter.h 0065745
> autotests/testincidenceformatter.cpp 28dd952
> scripts/extract_strings_ki18n.py PRE-CREATION
> scripts/grantlee_strings_extractor.py PRE-CREATION
> src/CMakeLists.txt 0d7bdcc
> src/Messages.sh 5f5de62
> src/grantlee_plugin/CMakeLists.txt PRE-CREATION
> src/grantlee_plugin/datetimefilters.h PRE-CREATION
> src/grantlee_plugin/datetimefilters.cpp PRE-CREATION
> src/grantlee_plugin/icon.h PRE-CREATION
> src/grantlee_plugin/icon.cpp PRE-CREATION
> src/grantlee_plugin/kcalendargrantleeplugin.h PRE-CREATION
> src/grantlee_plugin/kcalendargrantleeplugin.cpp PRE-CREATION
> src/grantleeki18nlocalizer.cpp PRE-CREATION
> src/grantleeki18nlocalizer_p.h PRE-CREATION
> src/grantleetemplatemanager.cpp PRE-CREATION
> src/grantleetemplatemanager_p.h PRE-CREATION
> src/incidenceformatter.cpp c58e961
> templates/CMakeLists.txt PRE-CREATION
> templates/attendee_row.html PRE-CREATION
> templates/event.html PRE-CREATION
> templates/freebusy.html PRE-CREATION
> templates/incidence_header.html PRE-CREATION
> templates/journal.html PRE-CREATION
> templates/template_base.html PRE-CREATION
> templates/todo.html PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/125331/diff/
>
>
> Testing
> -------
>
> I can see my events.
>
>
> Thanks,
>
> Daniel Vrátil
>
>
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/
More information about the kde-pim
mailing list