[Kde-pim] Review Request 125331: Port part of KCalUtils::IncidenceFormatter to Grantlee template system

Sandro Knauß sknauss at kde.org
Mon Oct 5 16:39:51 BST 2015


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125331/#review86382
-----------------------------------------------------------


additional tests:

* incidence exceptions (thisandfuture and simple)
* complete day, dateonly vs. datetime, multiday


src/grantlee_plugin/icon.cpp (line 116)
<https://git.reviewboard.kde.org/r/125331/#comment59499>

    add a "alt" propery for screenreader or people who can't look pictures.



src/grantleetemplatemanager.cpp (line 104)
<https://git.reviewboard.kde.org/r/125331/#comment59506>

    empty line -> delete



src/grantleetemplatemanager.cpp (line 115)
<https://git.reviewboard.kde.org/r/125331/#comment59507>

    two empty lines



src/incidenceformatter.cpp (line 328)
<https://git.reviewboard.kde.org/r/125331/#comment59497>

    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.



src/incidenceformatter.cpp (line 718)
<https://git.reviewboard.kde.org/r/125331/#comment59498>

    why do you need to differ between oneday one multiday Freebusy here? for todo/events it was not needed



templates/event.html (line 174)
<https://git.reviewboard.kde.org/r/125331/#comment59500>

    i18np "Category:" "Categories:" ...



templates/event.html (line 182)
<https://git.reviewboard.kde.org/r/125331/#comment59501>

    i18np "Attachment:" "Attachments:" ...



templates/journal.html (line 35)
<https://git.reviewboard.kde.org/r/125331/#comment59502>

    i18np



templates/todo.html (line 156)
<https://git.reviewboard.kde.org/r/125331/#comment59503>

    i18np



templates/todo.html (line 156)
<https://git.reviewboard.kde.org/r/125331/#comment59504>

    i18np



templates/todo.html (line 185)
<https://git.reviewboard.kde.org/r/125331/#comment59505>

    i18np


- Sandro Knauß


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