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

Closes #5503: ISO 8601 date in UI and alternative format as tooltip #5764

Conversation

ypid
Copy link
Contributor

@ypid ypid commented Feb 7, 2021

Fixes: #5503

With this PR all dates in the UI are now consistently displayed.

I changed the long date format as suggested by @xkilian and confirmed by my own research.

  • DATETIME_FORMAT
    • Before: July 20, 2020 4:52 p.m.
    • Now: 20th July, 2020 16:52

"20th July, 2020" would be spoken as "the 20th of July, 2020" but the "the" and "of" are never written.

I also switch from the illogical a.m./p.m. time format to military time format that can be pronounced as "sixteen hundred and fifty-two".

@DanSheps
Copy link
Member

There seems to be a lot of repetition here, do you think it might be possible to create an include and pass the date, it would also make future modifications easier.

@ypid ypid force-pushed the feature/5503-ui-iso-date-with-tooltip branch from f3aba08 to 3db3c14 Compare February 11, 2021 20:56
@ypid
Copy link
Contributor Author

ypid commented Feb 11, 2021

Thanks for the pointer (am was/am new to Django), done.

The only place (that I know of, and I also greped the source and so on) with dates without tooltip is object_list.html. I tried it but there it does not work so easily because the dates are passed to Jinja as SafeString. Also, I am not sure if it would be good UI design to have tooltips in the list.

@ypid ypid force-pushed the feature/5503-ui-iso-date-with-tooltip branch from 3db3c14 to e17417b Compare April 24, 2021 20:10
@ypid
Copy link
Contributor Author

ypid commented Apr 24, 2021

I rebased and updated this patch to the latest develop branch. Some new date fields were introduced with NetBox 2.11. I hope I found all of them by greping the source. If not, it is not the end of the world. I will come across them eventually and fix them (potentially in a new PR). I applied this patch (including the dependency patch 2618dde) on my instance now for testing during regular use. I tested all modified pages manually already. Feel free to have a look.

@ypid ypid force-pushed the feature/5503-ui-iso-date-with-tooltip branch from e17417b to 6e89d6b Compare May 2, 2021 11:19
@ypid
Copy link
Contributor Author

ypid commented May 19, 2021

I have been using this ever since and did not run into dates that I missed nor other issues. I would say this is ready. @jeremystretch Can you have a look?

@jeremystretch
Copy link
Member

@ypid Adding inclusion tags to display a value (i.e. {% include 'inc/date_now_span.html' %}) should be considered poor practice. Instead, we should use a template filter for this (e.g. {{ mydate|with_current_time }}). We should also only need the one filter; the parse_date() and datatype_str() are unnecessary since we can process the value directly.

@ypid ypid force-pushed the feature/5503-ui-iso-date-with-tooltip branch 2 times, most recently from 36cfa0c to 01c7d50 Compare May 26, 2021 20:09
@ypid
Copy link
Contributor Author

ypid commented May 26, 2021

I improved it, thanks. I agree with you that your approach is better. Now with the date_span filter, it is very easy to just append that if new date fields are added.

netbox/netbox/configuration.example.py Outdated Show resolved Hide resolved
netbox/templates/base.html Outdated Show resolved Hide resolved
netbox/utilities/templatetags/helpers.py Outdated Show resolved Hide resolved
netbox/utilities/templatetags/helpers.py Outdated Show resolved Hide resolved
@ypid ypid force-pushed the feature/5503-ui-iso-date-with-tooltip branch 2 times, most recently from 27738bd to 3378d51 Compare June 19, 2021 18:46
@ypid ypid force-pushed the feature/5503-ui-iso-date-with-tooltip branch 2 times, most recently from 8aa427f to 95b2c98 Compare July 2, 2021 20:22
ypid and others added 2 commits July 2, 2021 22:22
…mat as tooltip

With this commit all dates in the UI are now consistently displayed.

I changed the long date format as suggested by @xkilian and confirmed by my own
research.

* DATETIME_FORMAT
 * Before July 20, 2020 4:52 p.m.
 * Now 20th July, 2020 16:52

"20th July, 2020" would be spoken as "the 20th of July, 2020" but the "the" and
"of" are never written.

The only exception is `object_list.html`. I tried it but there it does not
work so easily because the dates are passed to Jinja as SafeString.
This changes the text from: Updated 5 months, 1 week ago
to: Updated 2021-01-24 00:33 (5 months, 1 week ago)

Co-authored-by: Jeremy Stretch <jstretch@ns1.com>
@ypid ypid force-pushed the feature/5503-ui-iso-date-with-tooltip branch from 95b2c98 to f331f10 Compare July 2, 2021 20:22
@jeremystretch:

> It'd be better to have the custom field return a date object than to
> accommodate string values in the template filter. Let's just omit custom
> field dates for now to keep this from getting any more complex.
@ypid ypid force-pushed the feature/5503-ui-iso-date-with-tooltip branch from f331f10 to a479c86 Compare July 2, 2021 20:30
@ypid
Copy link
Contributor Author

ypid commented Jul 2, 2021

Done.

@jeremystretch jeremystretch merged commit eaf0259 into netbox-community:develop Jul 6, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2022
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.

Default to ISO 8601 date format in UI and add "June 26, 2016" as tooltip
3 participants