date:20151030

Re: Review Request 125817: Add plugin system for Calendar events

2015-10-30 Thread Martin Klapetek



> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote:
> > src/declarativeimports/calendar/daysmodel.h, line 44
> > 
> >
> > I think we need some sort of
> > 
> > QStringList availablePlugins()
> > 
> > setPlugins( QStringList plugins);

What for?


> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote:
> > src/declarativeimports/calendar/daysmodel.cpp, line 63
> > 
> >
> > who deletes it?

Right, needs a qDeleteAll in ~


> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote:
> > src/declarativeimports/calendar/daysmodel.cpp, line 167
> > 
> >
> > Are you sure this works?
> > 
> > From what I see you're copying the event, then replacing the copy.
> > 
> > (though the autos make it confusing, so I may be wrong)

Yes, it does work. This looks through the stored events and replaces the stored 
one with the one that is passed as the method argument.


> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote:
> > src/declarativeimports/calendar/daysmodel.cpp, line 179
> > 
> >
> > this can be an invalid index, best to check it
> > 
> > (in each case we do this)

But then all it does is emit dataChanged() with invalid signal...but I'll add a 
check anyway.


> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote:
> > src/declarativeimports/calendar/daysmodel.cpp, line 209
> > 
> >
> > who owns this?
> > 
> > Q_INVOKABLE tries to be clever, which basically means I never know.
> > 
> > If QML owns it (which I think it does) you're potentially returning 
> > null values if this is called twice
> > 
> > if you own it, you're leaking this if you have m_qmlData when the model 
> > is deleted

Ah, good spot. Should be owned by the model, ideally. All problems solved.


> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote:
> > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h,
> >  line 173
> > 
> >
> > Unless needed otherwise, I'd put the UID in the constructor.
> > 
> > If a plugin uses this, you screw everything up

> If a plugin uses this, you screw everything up

I don't follow. Note that uid is not a mandatory property. You can have events 
without uid. As the comment above it says, it's useful only if you want to 
implement eventModified/eventRemoved (so that it can locate the events in the 
model), which in case of eg. holidays is not needed, cause they are not going 
to be modified/removed. So there's no need to make those plugins more complex 
for no reason.


> On Oct. 31, 2015, 2:08 a.m., David Edmundson wrote:
> > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h,
> >  line 202
> > 
> >
> > rather than this much docs, would it make sense to have a protected 
> > method that takes a list, then in this class you turn that into the hash?

Well if the only reason to do that is because "there's too much to read", then 
no :P


- Martin


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


On Oct. 28, 2015, 6:18 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125817/
> ---
> 
> (Updated Oct. 28, 2015, 6:18 p.m.)
> 
> 
> Review request for Plasma and Daniel Vrátil.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> This adds a simple plugin interface that can be subclassed
> and provide events integration with Plasma Calendar applet.
> 
> It's asynchronous and I've kept it deliberately simple.
> For now the Calendar tells the plugins which date range
> is being displayed, the plugins load the data and then
> emit the dataReady() signal containing the events.
> 
> The events are stored in a multihash for quick access
> by the Calendar's agenda part but also for overall
> easy-to-use (eg. in teh model data()).
> 
> The event data is stored in EventData class, which has
> a pretty self-explanatory members, except perhaps the
> "isMinor" one. The intention with this is to support
> namedays, where in some countries the calendars have
> different name every day. This is just a minor holiday
> and as such should not mark t

Re: Review Request 125817: Add plugin system for Calendar events

2015-10-30 Thread David Edmundson


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



src/declarativeimports/calendar/calendarplugin.cpp (line 36)


RegisterUncreatableType?



src/declarativeimports/calendar/daysmodel.h (line 44)


I think we need some sort of

QStringList availablePlugins()

setPlugins( QStringList plugins);



src/declarativeimports/calendar/daysmodel.cpp (line 33)


const



src/declarativeimports/calendar/daysmodel.cpp (line 35)


const



src/declarativeimports/calendar/daysmodel.cpp (line 63)


who deletes it?



src/declarativeimports/calendar/daysmodel.cpp (line 157)


Are you sure this works?

From what I see you're copying the event, then replacing the copy.

(though the autos make it confusing, so I may be wrong)



src/declarativeimports/calendar/daysmodel.cpp (line 169)


this can be an invalid index, best to check it

(in each case we do this)



src/declarativeimports/calendar/daysmodel.cpp (line 199)


who owns this?

Q_INVOKABLE tries to be clever, which basically means I never know.

If QML owns it (which I think it does) you're potentially returning null 
values if this is called twice

if you own it, you're leaking this if you have m_qmlData when the model is 
deleted



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 (line 173)


Unless needed otherwise, I'd put the UID in the constructor.

If a plugin uses this, you screw everything up



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 (line 180)


I'd add a dpointer here, even though you don't use it yet.



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 (line 202)


rather than this much docs, would it make sense to have a protected method 
that takes a list, then in this class you turn that into the hash?


- David Edmundson


On Oct. 28, 2015, 5:18 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125817/
> ---
> 
> (Updated Oct. 28, 2015, 5:18 p.m.)
> 
> 
> Review request for Plasma and Daniel Vrátil.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> This adds a simple plugin interface that can be subclassed
> and provide events integration with Plasma Calendar applet.
> 
> It's asynchronous and I've kept it deliberately simple.
> For now the Calendar tells the plugins which date range
> is being displayed, the plugins load the data and then
> emit the dataReady() signal containing the events.
> 
> The events are stored in a multihash for quick access
> by the Calendar's agenda part but also for overall
> easy-to-use (eg. in teh model data()).
> 
> The event data is stored in EventData class, which has
> a pretty self-explanatory members, except perhaps the
> "isMinor" one. The intention with this is to support
> namedays, where in some countries the calendars have
> different name every day. This is just a minor holiday
> and as such should not mark the calendar grid, otherwise
> the whole grid would be in a different color.
> 
> Putting the interface here might raise the question of
> depending on plasma-framework, but plugins provided by
> KDE can go to plasma-workspace and other 3rd party ones
> would just have to live with it. I don't think it will
> be a problem but if it turns out it is, we can rethink
> the placement.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/calendar/CMakeLists.txt 40ead91 
>   src/declarativeimports/calendar/calendarplugin.cpp bafe80c 
>   src/declarativeimports/calendar/daysmodel.h a5bdac9 
>   src/declarativeimports/calendar/daysmodel.cpp 2d059a8 
>   src/declarativeimports/calendar/eventdatadecorator.h PRE-CREATION 
>   src/declarativeimports/calendar/eventdatadecorator.cpp PRE-CREATION 
>   src/declarativeimports/calendar/plasmacalendarintegration/CMakeLists.txt 
> PRE-CREATION 
>   
> src/declarativeimports/calendar/plasmacalendarintegration/PlasmaCalendarIntegrationConfig.cmake.in
>  PR

[Powerdevil] [Bug 354623] New: Full charge not trigger notification

2015-10-30 Thread Anthony via KDE Bugzilla

https://bugs.kde.org/show_bug.cgi?id=354623

Bug ID: 354623
   Summary: Full charge not trigger notification
   Product: Powerdevil
   Version: 5.4.2
  Platform: Archlinux Packages
OS: Linux
Status: UNCONFIRMED
  Severity: normal
  Priority: NOR
 Component: general
  Assignee: plasma-devel@kde.org
  Reporter: bvb...@abv.bg

When battery is full charged, notification is not shown.

Reproducible: Always

Steps to Reproduce:
1. Systemsettings
2. Notification
3. Power Management System
4. Charge complete
5. notification or task bar entry

Actual Results:  
Notification is not triggered on full charge

Expected Results:  
Notification on battery is full charge

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 125878: fix lost QuickViewSharedEngine initialSize on initializing

2015-10-30 Thread Takahiro Hashimoto


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

(Updated Oct. 30, 2015, 9:06 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma, David Edmundson and David Faure.


Changes
---

Submitted with commit 4ca3237c2e07ac035104af5730bd209c18f61fe2 by Takahiro 
Hashimoto to branch master.


Repository: kdeclarative


Description
---

After my commit 1c3f42afe17cc090372eaf45647d71495de12c19, view's initialiSize 
value is lost (remaining 0,0 in spite of item's size). It should be set by 
item's size for some reusing purpose (with initialSize() getter) 

Sorry again my bad commit :(


Diffs
-

  src/quickaddons/quickviewsharedengine.cpp 323cf92 

Diff: https://git.reviewboard.kde.org/r/125878/diff/


Testing
---

build and run test

kenya888@xps13 ~/devel/kf5/build/frameworks/kdeclarative $ ninja test
[1/1] Running tests...
Test project /home/kenya888/devel/kf5/build/frameworks/kdeclarative
Start 1: fullmodelaccesstest
1/2 Test #1: fullmodelaccesstest ..   Passed0.05 sec
Start 2: quickviewsharedengine
2/2 Test #2: quickviewsharedengine    Passed0.39 sec

100% tests passed, 0 tests failed out of 2

Total Test time (real) =   0.44 sec

--

confirmed ksplashqml works well which use QuickViewSharedEngine.


Thanks,

Takahiro Hashimoto

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 125864: Add back the Agenda part of the Calendar

2015-10-30 Thread Martin Klapetek

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

(Updated Oct. 30, 2015, 8:11 p.m.)

Review request for Plasma.

Changes
---

+ fix issues
+ add time label to the events, see new screenshot

I'm not entirely sure about the offset of the label when there is just one time
(the todo in the screenshot)
but it makes all things with time aligned properly, so I dunno. Thoughts?

Repository: plasma-workspace

Description
---

Adds back the Agenda that was there pre-5.0 with some changes.

It now shows an overview of the given day events/holidays. I'd
like to add a second view at some point that would show you the
selected day by hours but that is much more work and the freeze
is getting closer, so maybe that will have to wait till 5.6.

Diffs (updated)
-

applets/digital-clock/package/contents/ui/CalendarView.qml b52eb10

Diff: https://git.reviewboard.kde.org/r/125864/diff/

Testing
---

Selecting a day correctly changes the date in the agenda and correctly displays
the events of the day.

Event removal/modification also removes/modifies it from the agenda.

File Attachments (updated)

Current look in Breeze

https://git.reviewboard.kde.org/media/uploaded/files/2015/10/29/5485250c-8d20-409e-8a5b-0eda7a9d7b8d__snapshot29.png
Multiple same-day events

https://git.reviewboard.kde.org/media/uploaded/files/2015/10/29/d453a01e-f9cf-4da3-8f6a-fac5aa6aadf5__snapshot18.png
New screenshot

https://git.reviewboard.kde.org/media/uploaded/files/2015/10/30/c75fed5e-02fd-4906-8bd9-726f0fc54ce8__snapshot30.png

Thanks,

Martin Klapetek

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Re: Review Request 125864: Add back the Agenda part of the Calendar

2015-10-30 Thread Martin Klapetek



> On Oct. 29, 2015, 5:55 p.m., Sebastian Kügler wrote:
> > applets/digital-clock/package/contents/ui/CalendarView.qml, line 46
> > 
> >
> > Could you clean this out before committing as well? (I know, nothing to 
> > do with your patch, just noticed it.)

I plan to do a bigger cleanup altogether, I'll make as part of that.


- Martin


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


On Oct. 29, 2015, 5:24 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125864/
> ---
> 
> (Updated Oct. 29, 2015, 5:24 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Adds back the Agenda that was there pre-5.0 with some changes.
> 
> It now shows an overview of the given day events/holidays. I'd
> like to add a second view at some point that would show you the
> selected day by hours but that is much more work and the freeze
> is getting closer, so maybe that will have to wait till 5.6.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml b52eb10 
> 
> Diff: https://git.reviewboard.kde.org/r/125864/diff/
> 
> 
> Testing
> ---
> 
> Selecting a day correctly changes the date in the agenda and correctly 
> displays the events of the day.
> 
> Event removal/modification also removes/modifies it from the agenda.
> 
> 
> File Attachments
> 
> 
> Current look in Breeze
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/29/5485250c-8d20-409e-8a5b-0eda7a9d7b8d__snapshot29.png
> Multiple same-day events
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/29/d453a01e-f9cf-4da3-8f6a-fac5aa6aadf5__snapshot18.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

42 matches

Mail list logo