-
Notifications
You must be signed in to change notification settings - Fork 278
Adds a DatePicker to EditView.xaml for the End Date. #4804
base: master
Are you sure you want to change the base?
Conversation
@atsvirchkova @skel35 I noticed other PRs ahead of mine have already been reviewed and merged - so I was wondering where in your review pipeline (or kanban board) this PR is currently at. Thanks! |
Is this project dead? |
Hi @Jehoel @rzfzr! |
@skel35 no worries, I cancelled my subscription to Toggl out of disgust for your company's contempt, or at least diregard, for open-source contributors ages ago :) |
@skel35 thanks for the quick reply! Would you be so kind to put this "crucial bug fixing" disclaimer first thing in the main Readme? Just feels fair, every non accepted PR is a competent someone that likes this product so much that they took time out of their day, and waste it... that's the recipe for getting reactions like Jehoel's... |
@rzfzr You're right that this information should've been shared publicly earlier. Thank you for the feedback. I've added a disclaimer to the top of the repo's Readme. |
📒 Description
This change adds a second
<DatePicker>
toEditView.xaml
which displays the Date component of an event's end timestamp and integrates it with EditView's plumbing.For my motivation, see https://github.com/toggl-open-source/toggldesktop/issues/2582
🕶️ Types of changes
New feature (non-breaking change which adds functionality)
🤯 List of changes
The
EditView.xaml
now has this layout and UI:EditView.xaml.cs
is updated with changes to integrate the End DatePicker.Also fixes build issues with the Windows project by specifying updated paths for the Windows 10 SDK's
*.winmd
files. Other users can add their own paths as well, if necessary, without breaking the<HintPath>
for other users.👫 Relationships
https://github.com/toggl-open-source/toggldesktop/issues/2582
Closes #2582
🔎 Review hints
I did some cursory testing for both Existing and New entries. I ran all tests in the solution - I wanted to add some UI integration tests for this change but I didn't see any, unfortunately. I did have unit test failures that were unrelated to my changes.
I'm perplexed by the design of Toggl's native API - I don't know why it treats the end-time as a localized time-only string instead of passing around only UNIX timestamp integers, that would make the Toggl API much simpler to work with and easier to understand.