Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Week number will be changed automatic now #5

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Week number will be changed automatic now #5

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions contents/ui/CompactRepresentation.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,30 @@ import QtQuick 2.0
import QtQuick.Layouts 1.1
import org.kde.plasma.components 2.0 as PlasmaComponents
import org.kde.plasma.calendar 2.0 as PlasmaCalendar
import ApplicationLauncher 1.0

Application {
id: launch
appName: PlasmaCalendar.Calendar.calendarBackend;
}
Timer {
interval: 500
Copy link
Owner

@anselmolsm anselmolsm Jun 7, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is the best solution. Consider this, it is a timer triggered every 500 ms to update a value that changes once a week, i.e. the timer is triggered twice per second, 2x60=120 per minute, 120*60=7200 per hour ... 1209600 per week.

I would go for a different solution. calendarBackend exposes this C++ API: https://api.kde.org/frameworks/plasma-framework/html/calendar_8h_source.html . currentWeek is not a QPROPERTY, it is just a QML invokable, so we can't rely on something like a currentWeekChanged signal. However, there are other signals that could help us to update the week number in a more effective way.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you fixed that?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got no time for this, too many life changes since #4 was filled. I left this PR open with hopes to get an update from the author, and that did not happen. If you manage to propose a better patch, please do so.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @anselmolsm. First of all, thank you for this plasmoid. I just started using it and I find it very handy on my work laptop.

It would be very nice if this bug was fixed. How about switching the delay to 1 update/minute? Surely this is nothing on modern computers.

I think it would be better to merge this and have the plasmoid update in a suboptimal way than let it show last week's number on a Monday afternoon :)

running: true;
repeat: true;
onTriggered: label.text = label.prefix+calendarBackend.currentWeek();
}

PlasmaComponents.Label {
id: label

text: ""

property string prefix: plasmoid.configuration.customPrefix

PlasmaCalendar.Calendar {
id: calendarBackend
}
horizontalAlignment: Text.AlignHCenter
verticalAlignment: Text.AlignVCenter
text: label.prefix + currentWeek()

function currentWeek() {
// Sunday & First 4-day week == ISO-8601, which is followed by Qt
Expand Down