feat: friendly number handling for note erasure timeouts #1163
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
(it does not make sense yet, to expose these as i18n, as I will be refactoring this whole thing into a standalone widget)
since we now don't erase after "x" seconds, we need to change the message
…ut-ui ; Conflicts: ; src/public/translations/ro/translation.json
eliandoran
approved these changes
Feb 13, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi,
this PR adds the possibility to set the note erasure timeout in a little bit more "UX friendly" way – people can select the time scale that they want to enter the number in.
See also #1136
Previously we were limited to entering values in "Seconds" only.
Internally the time is still stored in seconds , but for the frontend part, we do now also save the selected "timeScale" option (Seconds, Minutes, Hours, Days).
This "timeScale" is used to calculate the "user friendly" display value from the internal value.
This calculation involves some
Math.ceilbusiness, to avoid weird decimal numbers, which would be "not user friendly".For example let's say user enters "18 hours".
Internally this would now be stored as "64800" seconds.
If the person then changes the time scale to "Days", this will trigger a recalculation that would result in a fraction, because 18 hours (64800 sec) are not a full day (86400 sec):
64800 seconds / 86400 seconds = 0.75 Days
Since this would reintroduce the same issue we try to avoid, we run this value through Math.ceil to get the next largest integer, which is 1 in this case.
This in turn causes the "internal seconds value" to be recalculated to match the display value of "1 Day" - i.e. the value gets updated from the originally set 64800 seconds to 86400.
Backwards compatibility
Backwards compatibility is partially built-in:
The
eraseEntitiesAfterTimeScaleoption will be set to default to 86400 seconds (so using "Days" as timeScale).If the user has never changed the
eraseEntitiesAfterTimevalue, this will transparently be converted into 7 Days in the frontend and the actual value oferaseEntitiesAfterTimeis fully unaffected.If the user has changed the eraseEntitiesAfterTime, and that value e.g. is not calculating into a full day (see above example) - the value will be changed to match Days - i.e. the user might end up with a slightly "longer" note erasure timeout than they have set.
I think it would be best to just mention this in the release notes, rather than trying to add code to do some detection of what value the user currently have set.
Heads up:
this is essentially a "temporary" solution: ⇒ I will ultimately need to refactor this whole thing into a standalone widget, because we need the exact same logic at least for another 2 option widgets, but I need to have a thought about the best way to avoid as much code duplication as possible :-)
(but work is under way already)
The introduced option names will be unaffected by this refactoring.
The refactoring is also why I did not i18n some of the messages, as they would be uselessly translated for short period of time.
However I did change the wording in the existing i18n messages.