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

Jump to bottom

ADAPT-2752: Change date string format #204

Merged
merged 1 commit into from
Jun 14, 2021
Merged

ADAPT-2752: Change date string format #204

merged 1 commit into from
Jun 14, 2021

Conversation

Lyafel
Copy link
Collaborator

@Lyafel Lyafel commented Jun 11, 2021

NOT READY FOR REVIEW

  • (Edit the above to reflect status)

Summary

  • TL;DR - what's this PR for?

Review By (Date)

  • When does this need to be reviewed by?

Criticality

  • How critical is this PR on a 1-10 scale? Also see Severity Assessment.
  • E.g., it affects one site, or every site and product?

Review Tasks

Setup tasks and/or behavior to test

  1. Check out this branch
  2. Navigate to...
  3. Verify...

Front End Validation

  • Is the markup using the appropriate semantic tags and passes HTML validation?
  • Cross-browser testing has been performed?
  • Automated accessibility scans performed?
  • Manual accessibility tests performed?
  • Design is approved by @ user?

Backend / Functional Validation

Code

  • Are the naming conventions following our standards?
  • Does the code have sufficient inline comments?
  • Is there anything in this code that would be hidden or hard to discover through the UI?
  • Are there any code smells?
  • Are tests provided? eg (unit, behat, or codeception)

Code security

General

  • Is there anything included in this PR that is not related to the problem it is trying to solve?
  • Is the approach to the problem appropriate?

Affected Projects or Products

  • Does this PR impact any particular projects, products, or modules?

Associated Issues and/or People

  • JIRA ticket(s)
  • Other PRs
  • Any other contextual information that might be helpful (e.g., description of a bug that this PR fixes, new functionality that it adds, etc.)
  • Anyone who should be notified? (@mention them here)

Resources

@Lyafel Lyafel requested a review from yvonnetangsu June 11, 2021 16:55
@Lyafel
Copy link
Collaborator Author

Lyafel commented Jun 11, 2021

@yvonnetangsu Looks like YYYY-DD-MM is not a valid format in safari and your solutions works great! Just wanted to make sure that it's intended for the date to be treated as utc time? Should there be some sort of clarification for the content creators?

@yvonnetangsu
Copy link
Member

@yvonnetangsu Looks like YYYY-DD-MM is not a valid format in safari and your solutions works great! Just wanted to make sure that it's intended for the date to be treated as utc time? Should there be some sort of clarification for the content creators?

@Lyafel Glad that fix works!

And good point about the UTC time 😄 The published date isn't a very important item on this site (the stories aren't particularly time sensitive), and our clients haven't brought that up as the issue. Let's scope this ticket to just fixing the invalid date bug, and if the clients raise the issue about timezone later on, we can deal with that then. Thanks for asking!

yvonnetangsu
yvonnetangsu reviewed Jun 11, 2021
View reviewed changes
src/components/page-types/story/oodStoryFullView.js
Comment on lines +23 to +24
const startJsDateString = props.blok.publishedDate.replace(" ", "T");
const startUTCDate = new Date(`${startJsDateString}:00Z`);
Copy link
Member

Choose a reason for hiding this comment

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

Let's rename the variables to something a bit more relevant. Apologize I copied over the code from the other site where the time string is a start time. Let's replace "start" with "published" here. Thanks! After that should be good to go 😄

@yvonnetangsu
Copy link
Member

@Lyafel I'm going to go ahead and merge this in since it is working. Would be great if you could do the variable name tweak the campaign card PR 😄

yvonnetangsu
yvonnetangsu approved these changes Jun 14, 2021
View reviewed changes
Copy link
Member

@yvonnetangsu yvonnetangsu left a comment

Choose a reason for hiding this comment

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

GTG

@yvonnetangsu yvonnetangsu merged commit c03c2cc into main Jun 14, 2021
@yvonnetangsu yvonnetangsu deleted the ADAPT-2752-story-date branch June 14, 2021 21:18
yvonnetangsu added a commit that referenced this pull request Jun 15, 2021
@yvonnetangsu
Merge branch 'main' into ADAPT-2095-campaign
4f3a98d 
* main:
  ADAPT-2091: Quote card slideshow (#200)
  ADAPT-2752: Change date string format (#204)

# Conflicts:
#	src/scss/utilities/variables/_color-maps.scss
@Lyafel Lyafel mentioned this pull request Jun 22, 2021
Closed
14 tasks
yvonnetangsu added a commit that referenced this pull request Jun 23, 2021
@yvonnetangsu
Merge branch 'main' into ADAPT-2599-impact-1
c1b170b 
* main:
  ADAPT-2091: Quote card slideshow (#200)
  ADAPT-2752: Change date string format (#204)
  ADAPT-2558: Hero Component (#199)
  ADAPT-2727: Search button redesign (#203)

# Conflicts:
#	src/components/cards/oodCampaignCard.js
#	src/components/components.js
#	src/components/page-types/oodCampaignPage.js
#	src/scss/page-types/_campaign-page--fullwidth-image.scss
#	src/scss/page-types/_campaign-page--left-image.scss
#	src/scss/page-types/_campaign-page.scss
#	src/scss/utilities/variables/_color-maps.scss
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yvonnetangsu yvonnetangsu yvonnetangsu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Lyafel @yvonnetangsu