-
Notifications
You must be signed in to change notification settings - Fork 249
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.
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
Update widgets when task starts or becomes due #925
Update widgets when task starts or becomes due #925
Conversation
Otherwise a task would wrongly remain white if its view is updated exactly at the due date.
@@ -233,7 +233,7 @@ public RemoteViews getViewAt(int position) | |||
row.setTextViewText(android.R.id.text1, mDueDateFormatter.format(dueDate, DateFormatContext.WIDGET_VIEW)); | |||
|
|||
// highlight overdue dates & times | |||
if ((!dueDate.allDay && dueDate.before(mNow) || dueDate.allDay | |||
if ((!dueDate.allDay && Time.compare(dueDate, mNow) <= 0 || dueDate.allDay |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks unrelated to the issue. Why did you make this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While working on this fix I observed some cases where the update was processed right within the second of the due date. I noticed that in those cases the task did not turn red but remained white in the widget. That seemed wrong. The reason was that dueDate was not before but exactly equal to mNow – at least in android.text.format.Time
's second precision.
Therefore I changed this comparison essentially from dueDate < mNow
to dueDate <= mNow
. Sorry, if the commit message was not clear about my intention.
*/ | ||
public final class UpdateWidgetsAction implements TaskAction | ||
{ | ||
private final static String TAG = "UpdateWidgetsAction"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove unused field.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that was a relict of some debug logging. Removed.
public void execute(Context context, ContentProviderClient contentProviderClient, RowDataSnapshot<TaskContract.Instances> rowSnapshot, Uri taskUri) | ||
{ | ||
AppWidgetManager appWidgetManager = AppWidgetManager.getInstance(context); | ||
final ComponentName componentName = new ComponentName(context, TaskListWidgetProvider.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently we have two widget providers (mostly for backwards compatibility). Please ensure to update both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, did not see the other. Hope I got it right.
Thanks for the PR. Nice solution. I don't have any issues with using the notification actions. This mechanism is sort of proof of concept for a more generic plug-in mechanism. So it's nice to see we can use it for other use cases. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
This is an attempt to fix issue #17.
I hope it is not too bad style to talk to the widgets from the notification-related
ActionService
.