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

feat: human readable locktime duration for fidelity bonds #450

Merged
10 commits merged into from
Aug 5, 2022
Merged

Conversation

ghost
Copy link

@ghost ghost commented Aug 4, 2022

Resolves #397.

Mostly used existing code from the old fidelity bond branch. 👴

Moved out the time interval computation to an extra function but I don't really like it anymore since it gets super long at the call site. Also added a small addition to make sure to loop over the time units in the correct order, since it's not so clear if looping over Object.keys() always guarantees the order. So I figured manually sorting wouldn't hurt.

The tests are the same as on the old branch. I hope it works like this --might refactor the fb.time API tomorrow to make callsites a bit cleaner. 🧹

📸

Screenshot 2022-08-04 at 18 36 45

Screenshot 2022-08-04 at 18 36 49

Screenshot 2022-08-04 at 18 44 27

@ghost ghost added the UI/UX Issue related to cosmetics, design, or user experience label Aug 4, 2022
@ghost ghost requested a review from theborakompanioni August 4, 2022 16:58
@ghost ghost self-assigned this Aug 4, 2022
@ghost
Copy link
Author

ghost commented Aug 4, 2022

cc @ABHISHEK-PANDEY2 ☝️

@ghost ghost changed the title feat: human readable locktime duration for fidality bonds feat: human readable locktime duration for fidelity bonds Aug 4, 2022
Co-authored-by: Thebora Kompanioni <theborakompanioni@users.noreply.github.com>
Co-authored-by: Thebora Kompanioni <theborakompanioni@users.noreply.github.com>
Co-authored-by: Thebora Kompanioni <theborakompanioni@users.noreply.github.com>
Co-authored-by: Thebora Kompanioni <theborakompanioni@users.noreply.github.com>
@theborakompanioni
Copy link
Collaborator

theborakompanioni commented Aug 5, 2022

Nice!

Only thing missing is passing the locale. If the language is changed to French, the phrase is still in English, even when it would support the user's locale.

Something like:

const { i18n } = useTranslation()
[...]
fb.time.humanReadableDuration({ [...], locale: i18n.resolvedLanguage || i18n.language })

But maybe there is a better way?

Daniel and others added 3 commits August 5, 2022 12:02
Co-authored-by: Thebora Kompanioni <theborakompanioni@users.noreply.github.com>
Copy link
Collaborator

@theborakompanioni theborakompanioni left a comment

Choose a reason for hiding this comment

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

✔️ 💪

@ghost ghost merged commit 9d8e656 into master Aug 5, 2022
@ghost ghost deleted the fb-time-util branch August 5, 2022 10:28
@ghost ghost mentioned this pull request Aug 5, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
UI/UX Issue related to cosmetics, design, or user experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show total Fidelity Bond duration in addition to expiration date
1 participant