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.

Sign up for GitHub

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

Added datetime helper function #337

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

Added datetime helper function #337

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
905153d
Added datetime helper function
Monicaaawa Feb 19, 2023
1ed3d83
Merged datetime and timezone functions
Monicaaawa Feb 20, 2023
abd4f09
Create getDateTime function and add comments
Monicaaawa Apr 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/components/AnnouncementsPage/Announcement.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { makeStyles } from '@material-ui/core/styles';
import Container from '@material-ui/core/Container';
import Typography from '@material-ui/core/Typography';
import PropTypes from 'prop-types';
import { getDateTime } from '../../utils/datetime_utils';

const useStyles = makeStyles(theme => ({
subject: {
Expand All @@ -22,17 +23,13 @@ const useStyles = makeStyles(theme => ({
function Announcement({ subject, timestamp, body }) {
const classes = useStyles();

const d = new Date(timestamp);
const date = d.toLocaleDateString('en-US');
const time = d.toLocaleTimeString('en-US');

return (
<Container>
<Typography variant='h5' component='h2' className={classes.subject}>
{subject}
</Typography>
<Typography variant='subtitle1' className={classes.timestamp}>
Posted on {date} at {time} PST
Posted on {getDateTime(timestamp)} PST
</Typography>
<Typography variant='body2' className={classes.body}>
{body.trim()}
Expand Down
6 changes: 3 additions & 3 deletions src/components/HomePage/Banner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,10 +224,10 @@ function Banner() {
style={{ marginBottom: 10, fontWeight: 500 }}
component='h3'
>
<time dateTime={hothStart.toISOString()} hidden>
{month} {startDay}{endDayString}, 2022
<b>Date: </b><time dateTime={hothStart.toISOString()} >
{month} {startDay}{endDayString}, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not be hardcoding the year, but also getting the year from the hothStart date.

</time>
<b>Date:</b> Sunday, March 5, 2023
{/* <b>Date:</b> Sunday, March 5, 2023 */}
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are not using this code anymore, then you should delete it to keep our code clean.

</Typography>
</Tooltip>
</Box>
Expand Down
6 changes: 2 additions & 4 deletions src/components/HomePage/HomeAnnouncementBanner.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import data from '../../data/announcements.json';
import PropTypes from 'prop-types';
import Collapse from '@material-ui/core/Collapse';
import { Link } from 'gatsby';
import { getDateTime } from '../../utils/datetime_utils';

import ClearIcon from '@material-ui/icons/Clear';
import IconButton from '@material-ui/core/IconButton';
Expand Down Expand Up @@ -61,9 +62,6 @@ const useStyles = makeStyles(theme => ({

function HomeAnnouncement({ subject, timestamp, body }) {
const classes = useStyles();
const d = new Date(timestamp);
const date = d.toLocaleDateString('en-US');
const time = d.toLocaleTimeString('en-US', { hour: '2-digit', minute: '2-digit' });

return (
<Grid container direction='column' justify='space-between'>
Expand All @@ -76,7 +74,7 @@ function HomeAnnouncement({ subject, timestamp, body }) {
</Grid>
<Grid item>
<Typography variant='subtitle2' className={classes.date}>
{date}, {time}
{getDateTime(timestamp)}
</Typography>
</Grid>
</Grid>
Expand Down
10 changes: 10 additions & 0 deletions src/utils/datetime_utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export function getDateTime(timestamp) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Also, there seems to be a timezone_names.js file that has functionality similar to this file, so you should move the function in that file into this file and make this file the centralized util file that handles anything 'time' related.

try {
const d = new Date(timestamp);
const date = d.toLocaleDateString('en-US');
const time = d.toLocaleTimeString('en-US', { hour: '2-digit', minute: '2-digit' });
return `${date}, ${time}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't be returning a string with a specific format like ${date}, ${time} hardcoded in.
Consider the following use cases:

  • What if another component wants to use this format, but another component wants to display time like ${date} - ${time} instead?
  • Or what if a component doesn't care about the time, and only cares about the date?

This function should be able to handle either regardless. To fix this, we should instead return an object that maps keys to values so that each component can use the data however it wants. Like so:

return {
  date: date,
   time: time
}

And conveniently, javascript lets a simplify this code into the following that does the exact same thing:

return { date, time }

Then, in your components, you can use it like so:

const datetime = getDateTime(timestamp);
...

<p> Posted on {datetime.date}, {datetime.time} PST </p>

} catch {
return '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Return null instead, the default of an empty object

}
}