Skip to content
This repository has been archived by the owner on Jun 24, 2022. It is now read-only.

Now generating the expiry via javascript and showing local time #77

Merged
merged 1 commit into from
Jul 9, 2020

Conversation

PierrePaul
Copy link
Contributor

Now generating the expiry time in javascript and adding 24 h. Also displaying the local time in both french and english format.

Screenshot_2020-07-09 Fournir le numéro au patient


Screenshot_2020-07-09 Give patient this number

@PierrePaul PierrePaul requested a review from a team as a code owner July 9, 2020 04:08
@pcraig3 pcraig3 temporarily deployed to healthcare-p-bugfix-cod-z8hdom July 9, 2020 04:08 Inactive
@pcraig3 pcraig3 temporarily deployed to healthcare-p-bugfix-cod-z8hdom July 9, 2020 04:09 Inactive
@lgtm-com
Copy link

lgtm-com bot commented Jul 9, 2020

This pull request introduces 1 alert when merging f00935e into f3e254e - view on LGTM.com

new alerts:

  • 1 for Unused import

@pcraig3 pcraig3 temporarily deployed to healthcare-p-bugfix-cod-z8hdom July 9, 2020 05:03 Inactive
profiles/static/js/script.js Outdated Show resolved Hide resolved
profiles/templates/profiles/code.html Outdated Show resolved Hide resolved
Comment on lines 40 to 45
<script type="text/javascript">
document.addEventListener('DOMContentLoaded', function(){
const expiry = new Date().addHours(24);
document.getElementById('expiry').innerHTML = expiry.toLocaleString('{{ LANGUAGE_CODE }}');
}, false);
</script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, this looks clean.

We'd like to keep a similar format as we had before though (I know vanilla JS date formatting is awful), so I'm not sure what's the cheapest solution here.

Maybe pulling in date-fns from an CDN?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my first reflex, but the latest version of the (official) CDN for date-fns was 1.9 (4 years ago). unpkg does seem to have a good up to date version (2.14) so I will make the switch. Thanks!

@pcraig3
Copy link
Contributor

pcraig3 commented Jul 9, 2020

Heyo, we want more control over the format of the "expires" string.

What we have now in Figma is pretty different than we get here, so maybe bring in date-fns as a CDN dependency or maybe there's a better way to do this? I think we should stay away from npm if it's only for one JS dependency.

Screen Shot 2020-07-09 at 10 01 57 AM

Screen Shot 2020-07-09 at 10 01 46 AM

@PierrePaul
Copy link
Contributor Author

After talking with Paul, we had too many issues using date-fns without using NPM. So here is the updated pull request with the smallest version of momentjs, delivered by the cloudfare DNS.
The new format and their translations :
French
Screenshot_2020-07-09 Fournir le numéro au patient(2)


English
Screenshot_2020-07-09 Give patient this number(2)

@pcraig3 pcraig3 temporarily deployed to healthcare-p-bugfix-cod-z8hdom July 9, 2020 17:07 Inactive
@pcraig3 pcraig3 force-pushed the bugfix-code-expiry-local-time branch from e577634 to 32dc679 Compare July 9, 2020 19:40
@pcraig3 pcraig3 temporarily deployed to healthcare-p-bugfix-cod-z8hdom July 9, 2020 19:40 Inactive
The logic was making our template really complex, so pull it into
a separate includes file.

Did it progressively-enhanced, so if Javascript fails to run, it will
still show the time and date in EDT.
@pcraig3 pcraig3 force-pushed the bugfix-code-expiry-local-time branch from 32dc679 to 8493f3e Compare July 9, 2020 19:41
@pcraig3 pcraig3 temporarily deployed to healthcare-p-bugfix-cod-z8hdom July 9, 2020 19:41 Inactive
Copy link
Contributor

@pcraig3 pcraig3 left a comment

Choose a reason for hiding this comment

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

Updated it, moved the code to a new includes file. Gonna merge.

@pcraig3 pcraig3 merged commit 44a83de into main Jul 9, 2020
@pcraig3 pcraig3 deleted the bugfix-code-expiry-local-time branch July 9, 2020 19:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants