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.

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

WIP: Add Sec2TrackedTime time format template helper #25213

Open
wants to merge 38 commits into
base: main
Choose a base branch
from

Conversation

6543
Copy link
Member

@6543 6543 commented Jun 12, 2023

As work-days/-weeks/-years ... differ from country and company, its not practicable to use it as time format

TODOs:

  • migrate trackedTimeComments content -> to be timestamps and use helper to format
  • add UI to change user TT-Max-U setting
  • alter helper to be able to set max unit
    • frontend js helper
    • backend template helper
  • create new JS compontente "tracked time"
  • create a struct to forward user (apperance) settings to frontend (save in window.userSettings or so)
  • migrate in templates to compontente "tracked time"
  • optional: make frontent js tracked-time componente localize

Sponsored by Kithara Software GmbH

Sorry, something went wrong.

6543 added 3 commits June 12, 2023 21:52

Unverified

The email in this signature doesn’t match the committer email.
As work-days/-weeks/-years ... differ from country and company, its not practicable to use it as time format

Unverified

The email in this signature doesn’t match the committer email.

Unverified

The email in this signature doesn’t match the committer email.
@6543 6543 added type/enhancement An improvement of existing functionality outdated/theme/timetracker labels Jun 12, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jun 12, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jun 12, 2023
@6543 6543 added this to the 1.21.0 milestone Jun 12, 2023
@6543
Copy link
Member Author

6543 commented Jun 12, 2023

image


image

add

Unverified

The email in this signature doesn’t match the committer email.
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jun 12, 2023
@KN4CK3R
Copy link
Member

KN4CK3R commented Jun 12, 2023

If something took 3 days, why should that differ from country and company? Do you want to use the displayed value for billing?

Unverified

The email in this signature doesn’t match the committer email.
@6543
Copy link
Member Author

6543 commented Jun 12, 2023

@KN4CK3R yes

@6543
Copy link
Member Author

6543 commented Jun 12, 2023

I personally think that days only are counter-intuitive too. if you think a second time ... well but I'm happy to move it into a server config option through I think we can just separate time formation for tracked-time in any case

Con: we can have large h values
Pro: hours mean the same in any area and can be used to create bills

@KN4CK3R
Copy link
Member

KN4CK3R commented Jun 12, 2023

Then it's debatable if that's the common usecase. Other people want to say "this took me 3 days" without calculating the value. Similar to the "my child is 27 months old" people... well keep your secrets.

@6543
Copy link
Member Author

6543 commented Jun 12, 2023

well was it 3days non stop without sleep ;)

@6543
Copy link
Member Author

6543 commented Jun 12, 2023

well ... so should we go for a server config option?

@silverwind
Copy link
Member

This is only about tracked time, right? I guess you could have a TRACKED_TIME_UNIT option, default to any which renders in the highest unit, and have it settable to days, hours, minutes, seconds which would render in at most that unit. I think you also missed a few places in the UI, e.g. the dropdown in the header bar for active time tracker.

@6543 6543 added the pr/wip This PR is not ready for review label Jun 12, 2023
@6543

This comment was marked as outdated.

@6543 6543 changed the title Add Sec2TrackedTime time format template helper WIP: Add Sec2TrackedTime time format template helper Jun 12, 2023
@wxiaoguang
Copy link
Contributor

For i18n:

For template helper, if it really needs to add a function, it could be put into TimeUtils, like StringUtils/JsonUtils, to avoid bloating the template helpers.

@6543
Copy link
Member Author

6543 commented Jun 13, 2023

well I'll make backend just sends timestamps around ... and conversion can be done in frontend by mentioned browser apis ...

Unverified

The email in this signature doesn’t match the committer email.
@silverwind
Copy link
Member

I checked relative-time-element which has a format=duration option, but I think it lacks an option to show the value in a defined unit. precision only limits the smallest display unit.

Unverified

The email in this signature doesn’t match the committer email.

Unverified

The email in this signature doesn’t match the committer email.
@6543
Copy link
Member Author

6543 commented Jun 20, 2023

next pull towards 1. -> #25392

6543 added a commit that referenced this pull request Jun 23, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
this will allow us to fully localize it later

PS: we can not migrate back as the old value was a one-way conversion


prepare for  #25213

---
*Sponsored by Kithara Software GmbH*
6543 added 5 commits June 23, 2023 17:27

Unverified

The email in this signature doesn’t match the committer email.

Unverified

The email in this signature doesn’t match the committer email.
wip

Unverified

The email in this signature doesn’t match the committer email.

Unverified

The email in this signature doesn’t match the committer email.

Unverified

The email in this signature doesn’t match the committer email.
@silverwind
Copy link
Member

Still WIP?

@6543
Copy link
Member Author

6543 commented Aug 25, 2023

yes ... Just look at the TODO that's a bigger one :) and I work on it via payed time so it's not as fast as stuff I do in my free time :D (because upstreaming has not the highest priority there)

@6543 6543 modified the milestones: 1.21.0, 1.22.0 Sep 2, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
6543 and others added 5 commits October 21, 2023 00:05

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@vsysoev
Copy link
Contributor

vsysoev commented Dec 2, 2023

Hello, I believe that hour(minutes) are the most universal and cultural independent approach to understand amount of time spent for job to be done. That is why this is truly KISS approach. And not too much job to be done to implement this.

@lafriks
Copy link
Member

lafriks commented Dec 2, 2023

I also agree that for time spent hours should be the highest measurement unit, days and others can be misunderstood and can be confusing

@6543 6543 removed this from the 1.22.0 milestone Dec 3, 2023
@6543
Copy link
Member Author

6543 commented Dec 3, 2023

yes I still want the underlaying code to be cleaner (before i remove the wip) :)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@vsysoev
Copy link
Contributor

vsysoev commented Dec 3, 2023

yes I still want the underlaying code to be cleaner (before i remove the wip) :)

@6543 because we are doing same job and share same approach, I put my PR #28312 on WIP and wait you to finish yours.

@6543
Copy link
Member Author

6543 commented Dec 3, 2023

Haha ok :)

@wxiaoguang
Copy link
Contributor

@6543 Tracked time representation (added hours) might be modified with settings in UI section in app.ini #33315

@wxiaoguang
Copy link
Contributor

Replaced by Make tracked time representation display as hours #33315

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. outdated/theme/timetracker pr/wip This PR is not ready for review size/L Denotes a PR that changes 100-499 lines, ignoring generated files. type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants