Re: Review Request 118357: Disable the agenda part of the calendar

2014-06-13 Thread Martin Klapetek


> On June 12, 2014, 12:25 p.m., Vishesh Handa wrote:
> > I seem to have missed this change.
> > 
> > The rationale behind this change is that there is no PIM support and 
> > therefore we should remove it. I'm sure you guys would have thought about 
> > the following points, but since the discussion happened on IRC I cannot go 
> > through it.
> > 
> > 1. The Agenda part still provides valuable feedback such as the current 
> > date + day
> > 2. What about if someone doesn't have any accounts enabled in KDE PIM? Do 
> > we plan to toggle it on and off based on runtime detection?
> > 3. We clearly want the agenda in the future. Why break visual consistency 
> > among releases, specially since the work is done.
> > 4. From a promo point of view, we have been showcasing our calendar widget 
> > a LOT. It was the only thing that was shown during FOSDEM. Do we want to 
> > change that?
> 
> Martin Klapetek wrote:
> Hey,
> 
> thanks for that. I remember we also talked about disabling this part back 
> in January in case the pim backend would not be ready, but to your questions:
> 
> 1. The only thing it provided over the calendar side was the full day 
> name, which itself is not enough to keep the whole popup twice as big as 
> needed.
> 2. Possibly. Subject to discussion though.
> 3. I don't believe that in this case the "visual consistency among 
> releases" tops the "it's (visually) bloated for no reason". Also the visual 
> consistency of the calendar will remain in the future, it will just be 
> extended. Limiting ourselves to the very first visuals released in the first 
> release would be quite...limiting :) Plus we might also need to change the 
> agenda part in the future a bit, it wasn't really tested with real agenda 
> data afaik (and so the visual consistency might break anyway). And finally, 
> it may take several releases of Plasma before there's a usable backend. 
> Having that big calendar popup with half of it being pretty much just a 
> whitespace for so long would be just annoying.
> 4. Indeed, as it was the very first moreless complete applet of Plasma 
> Next. Just now we also have good NM applet and battery applet with new icons 
> worth showing
> 
> Vishesh Handa wrote:
> Just to be clear what we are discussing over here isn't a technical 
> argument. I get it, it doesn't work, we shouldn't ship it. I still think the 
> agenda should be there.
> 
> 1. How about the fact that it is much more readable and it looks much 
> better? If you think this is a subjective analysis, we can try to form a 
> general consensus among us developers. Or maybe ask the VDG.
> 
> 2. Alright. Lets ignore that for now.
> 
> 3. I think it's unfair to use the word bloat over here. I look more at it 
> as small iterations in the design. Adding an agenda later totally changes the 
> calendar. Making modifications to it, is just iterating over the initial 
> design. Also, by "among releases", I also meant wrt KDE4 -> 5. The Calendar 
> is KDE4 times looked similar to the one we have now with the agenda.
> 
> 4. Yes, we have other plasmoids, but that still doesn't change the case 
> that the Calendar plasmoid is what we showcased a LOT, and got good feedback 
> about. We're now changing that to something which may or may not look good 
> (See point (1)). Also, considering that amount of name changing we have done 
> (Plasma 2 -> Plasma Next -> Plamsa 5?) it seems sad that we are also changing 
> what most people saw as Plasma when we were initially showcasing it.

1. Subjective - a bit yeah. However, I was wondering (experimenting, even) 
about having the full date in the calendar header, something like "Today is 
Friday, 13th June 2014" (oh dayumn...spooky) and when you click any day in the 
calendar, it would change to just "Saturday, 14th June 2014". Maybe without the 
year. That would also fix the standalone calendar applet.

3. Now that's subjective (the "bloat") :) Right, I misunderstood the among 
releases then. Yeah it will change the calendar, but should we make it runtime 
detection based, it might not change for bigger part of users

4. "we showcased a LOT" -> yes, but because there was *nothing else* to 
showcase besides wallpaper and panel at that time. Now we have nice looking 
kickoff, nice looking analog clock, nice looking other applets which we can 
showcase. Plus I think that a significant portion of the good feedback was the 
wow factor of the new theme and the background contrast thing (which is what 
the calendar was used to demonstrate iirc).

---

We could possibly add the day part of the agenda back and only that part, make 
it a big bigger and get rid of the agenda lines. Might work out good + then it 
will be the "iteration design".


- Martin


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

Re: Review Request 118357: Disable the agenda part of the calendar

2014-06-13 Thread Vishesh Handa


> On June 12, 2014, 10:25 a.m., Vishesh Handa wrote:
> > I seem to have missed this change.
> > 
> > The rationale behind this change is that there is no PIM support and 
> > therefore we should remove it. I'm sure you guys would have thought about 
> > the following points, but since the discussion happened on IRC I cannot go 
> > through it.
> > 
> > 1. The Agenda part still provides valuable feedback such as the current 
> > date + day
> > 2. What about if someone doesn't have any accounts enabled in KDE PIM? Do 
> > we plan to toggle it on and off based on runtime detection?
> > 3. We clearly want the agenda in the future. Why break visual consistency 
> > among releases, specially since the work is done.
> > 4. From a promo point of view, we have been showcasing our calendar widget 
> > a LOT. It was the only thing that was shown during FOSDEM. Do we want to 
> > change that?
> 
> Martin Klapetek wrote:
> Hey,
> 
> thanks for that. I remember we also talked about disabling this part back 
> in January in case the pim backend would not be ready, but to your questions:
> 
> 1. The only thing it provided over the calendar side was the full day 
> name, which itself is not enough to keep the whole popup twice as big as 
> needed.
> 2. Possibly. Subject to discussion though.
> 3. I don't believe that in this case the "visual consistency among 
> releases" tops the "it's (visually) bloated for no reason". Also the visual 
> consistency of the calendar will remain in the future, it will just be 
> extended. Limiting ourselves to the very first visuals released in the first 
> release would be quite...limiting :) Plus we might also need to change the 
> agenda part in the future a bit, it wasn't really tested with real agenda 
> data afaik (and so the visual consistency might break anyway). And finally, 
> it may take several releases of Plasma before there's a usable backend. 
> Having that big calendar popup with half of it being pretty much just a 
> whitespace for so long would be just annoying.
> 4. Indeed, as it was the very first moreless complete applet of Plasma 
> Next. Just now we also have good NM applet and battery applet with new icons 
> worth showing

Just to be clear what we are discussing over here isn't a technical argument. I 
get it, it doesn't work, we shouldn't ship it. I still think the agenda should 
be there.

1. How about the fact that it is much more readable and it looks much better? 
If you think this is a subjective analysis, we can try to form a general 
consensus among us developers. Or maybe ask the VDG.

2. Alright. Lets ignore that for now.

3. I think it's unfair to use the word bloat over here. I look more at it as 
small iterations in the design. Adding an agenda later totally changes the 
calendar. Making modifications to it, is just iterating over the initial 
design. Also, by "among releases", I also meant wrt KDE4 -> 5. The Calendar is 
KDE4 times looked similar to the one we have now with the agenda.

4. Yes, we have other plasmoids, but that still doesn't change the case that 
the Calendar plasmoid is what we showcased a LOT, and got good feedback about. 
We're now changing that to something which may or may not look good (See point 
(1)). Also, considering that amount of name changing we have done (Plasma 2 -> 
Plasma Next -> Plamsa 5?) it seems sad that we are also changing what most 
people saw as Plasma when we were initially showcasing it.


- Vishesh


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


On June 2, 2014, 12:44 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated June 2, 2014, 12:44 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
>

Re: Review Request 118357: Disable the agenda part of the calendar

2014-06-13 Thread Martin Klapetek


> On June 12, 2014, 12:25 p.m., Vishesh Handa wrote:
> > I seem to have missed this change.
> > 
> > The rationale behind this change is that there is no PIM support and 
> > therefore we should remove it. I'm sure you guys would have thought about 
> > the following points, but since the discussion happened on IRC I cannot go 
> > through it.
> > 
> > 1. The Agenda part still provides valuable feedback such as the current 
> > date + day
> > 2. What about if someone doesn't have any accounts enabled in KDE PIM? Do 
> > we plan to toggle it on and off based on runtime detection?
> > 3. We clearly want the agenda in the future. Why break visual consistency 
> > among releases, specially since the work is done.
> > 4. From a promo point of view, we have been showcasing our calendar widget 
> > a LOT. It was the only thing that was shown during FOSDEM. Do we want to 
> > change that?

Hey,

thanks for that. I remember we also talked about disabling this part back in 
January in case the pim backend would not be ready, but to your questions:

1. The only thing it provided over the calendar side was the full day name, 
which itself is not enough to keep the whole popup twice as big as needed.
2. Possibly. Subject to discussion though.
3. I don't believe that in this case the "visual consistency among releases" 
tops the "it's (visually) bloated for no reason". Also the visual consistency 
of the calendar will remain in the future, it will just be extended. Limiting 
ourselves to the very first visuals released in the first release would be 
quite...limiting :) Plus we might also need to change the agenda part in the 
future a bit, it wasn't really tested with real agenda data afaik (and so the 
visual consistency might break anyway). And finally, it may take several 
releases of Plasma before there's a usable backend. Having that big calendar 
popup with half of it being pretty much just a whitespace for so long would be 
just annoying.
4. Indeed, as it was the very first moreless complete applet of Plasma Next. 
Just now we also have good NM applet and battery applet with new icons worth 
showing


- Martin


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


On June 2, 2014, 2:44 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated June 2, 2014, 2:44 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-06-12 Thread Vishesh Handa

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


I seem to have missed this change.

The rationale behind this change is that there is no PIM support and therefore 
we should remove it. I'm sure you guys would have thought about the following 
points, but since the discussion happened on IRC I cannot go through it.

1. The Agenda part still provides valuable feedback such as the current date + 
day
2. What about if someone doesn't have any accounts enabled in KDE PIM? Do we 
plan to toggle it on and off based on runtime detection?
3. We clearly want the agenda in the future. Why break visual consistency among 
releases, specially since the work is done.
4. From a promo point of view, we have been showcasing our calendar widget a 
LOT. It was the only thing that was shown during FOSDEM. Do we want to change 
that?

- Vishesh Handa


On June 2, 2014, 12:44 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated June 2, 2014, 12:44 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-06-02 Thread Martin Klapetek

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

(Updated June 2, 2014, 12:44 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-workspace


Description
---

As agreed on irc (someday ago), the agenda part is useless until there's a 
proper agenda backend and should just be hidden. Here's a patch simply hiding 
the left part - it's easier to do just "visible: false" than comment it out and 
then comment out/remove all the lines referencing parts of the agenda, also 
cleaner.

I added a big comment at the file beginning, I'll fill the commit sha after 
committing so it can be easily reverted (the comment will be separate commit).

Screenshot attached.

Oh and you might want to remove the clock from panel and readd it/remove plasma 
config as the popup size seems to be saved and so after this patch you may 
still get the original-sized half-empty dialog.


Diffs
-

  applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 

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


Testing
---


File Attachments


Calendar without agenda
  
https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png


Thanks,

Martin Klapetek

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-06-02 Thread Commit Hook

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


This review has been submitted with commit 
bfd62154d8e892d4fdc87d27d25d07cb7841c1e6 by Martin Klapetek to branch master.

- Commit Hook


On May 27, 2014, 4 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated May 27, 2014, 4 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-05-30 Thread Sebastian Kügler


> On May 30, 2014, 1:27 p.m., Sebastian Kügler wrote:
> > Since the visible flag is now static false, let's comment the agenda item 
> > code, so we don't waste memory or CPU cycles. Once done, please ship.
> 
> Martin Klapetek wrote:
> I specifically opted out of commenting it as that would mean commenting 
> stuff all around (stuff that references the commented out stuff). If really 
> needed, I can do tho. On the other hand, if this is to make things faster, it 
> will make them slow again in the future --> users will observe performance 
> drop ;)
> 
> Sebastian Kügler wrote:
> That's a pretty weak argument. "Let's keep rubbish in that isn't used, so 
> when we use it, it's as slow as it's now and nobody will notice that we added 
> stuff and slowed it down", essentially. :D
> 
> The agenda part should be optional in the code, perhaps show / hide 
> dynamically, based on whether the user wants or as agenda items. These are 
> quite complex bits of UI, the lazy-loading helps, but we should still attempt 
> to make it not load stuff that isn't necessary.
> 
> So let's do it right.

Ah, btw ... commenting stuff all around is not needed: You can just check 
within the code if references are valid and make it conditional, that's more 
future-proof as well.


- Sebastian


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


On May 27, 2014, 4 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated May 27, 2014, 4 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-05-30 Thread Sebastian Kügler


> On May 30, 2014, 1:27 p.m., Sebastian Kügler wrote:
> > Since the visible flag is now static false, let's comment the agenda item 
> > code, so we don't waste memory or CPU cycles. Once done, please ship.
> 
> Martin Klapetek wrote:
> I specifically opted out of commenting it as that would mean commenting 
> stuff all around (stuff that references the commented out stuff). If really 
> needed, I can do tho. On the other hand, if this is to make things faster, it 
> will make them slow again in the future --> users will observe performance 
> drop ;)

That's a pretty weak argument. "Let's keep rubbish in that isn't used, so when 
we use it, it's as slow as it's now and nobody will notice that we added stuff 
and slowed it down", essentially. :D

The agenda part should be optional in the code, perhaps show / hide 
dynamically, based on whether the user wants or as agenda items. These are 
quite complex bits of UI, the lazy-loading helps, but we should still attempt 
to make it not load stuff that isn't necessary.

So let's do it right.


- Sebastian


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


On May 27, 2014, 4 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated May 27, 2014, 4 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-05-30 Thread Martin Klapetek


> On May 30, 2014, 3:27 p.m., Sebastian Kügler wrote:
> > Since the visible flag is now static false, let's comment the agenda item 
> > code, so we don't waste memory or CPU cycles. Once done, please ship.

I specifically opted out of commenting it as that would mean commenting stuff 
all around (stuff that references the commented out stuff). If really needed, I 
can do tho. On the other hand, if this is to make things faster, it will make 
them slow again in the future --> users will observe performance drop ;)


- Martin


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


On May 27, 2014, 6 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated May 27, 2014, 6 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-05-30 Thread Sebastian Kügler

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

Ship it!


Since the visible flag is now static false, let's comment the agenda item code, 
so we don't waste memory or CPU cycles. Once done, please ship.

- Sebastian Kügler


On May 27, 2014, 4 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated May 27, 2014, 4 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-05-30 Thread Mark Gaiser

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

Ship it!


Ship it since the changes look sane to me.
Mind you, i'm not the author of this code.

- Mark Gaiser


On May 27, 2014, 4 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated May 27, 2014, 4 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-05-29 Thread Martin Klapetek

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


Ping?

- Martin Klapetek


On May 27, 2014, 6 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated May 27, 2014, 6 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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


Re: Review Request 118357: Disable the agenda part of the calendar

2014-05-27 Thread Aleix Pol Gonzalez

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


I agree it makes sense, +1. Not adding "ship it" because the original authors 
might have comments.

- Aleix Pol Gonzalez


On May 27, 2014, 4 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118357/
> ---
> 
> (Updated May 27, 2014, 4 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> As agreed on irc (someday ago), the agenda part is useless until there's a 
> proper agenda backend and should just be hidden. Here's a patch simply hiding 
> the left part - it's easier to do just "visible: false" than comment it out 
> and then comment out/remove all the lines referencing parts of the agenda, 
> also cleaner.
> 
> I added a big comment at the file beginning, I'll fill the commit sha after 
> committing so it can be easily reverted (the comment will be separate commit).
> 
> Screenshot attached.
> 
> Oh and you might want to remove the clock from panel and readd it/remove 
> plasma config as the popup size seems to be saved and so after this patch you 
> may still get the original-sized half-empty dialog.
> 
> 
> Diffs
> -
> 
>   applets/digital-clock/package/contents/ui/CalendarView.qml 43812a4 
> 
> Diff: https://git.reviewboard.kde.org/r/118357/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Calendar without agenda
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2014/05/27/84880c21-1367-4d48-88db-7747553c40f5__plasma_cal1.png
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

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