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

Fix release date issue in Safari #4067

Merged
merged 1 commit into from Oct 31, 2019
Merged

Conversation

Dananji
Copy link
Contributor

@Dananji Dananji commented Oct 7, 2019

Fixes #4005

When editing admin sets in Safari, the embargo release date format shows up as yyyy-mm-dd and is not able to save dates in the format mm/dd/yyyy (expected format) sometimes.

Changes proposed in this pull request:

  • Format date values on page load and on submit into desired date format.

Guidance for testing, such as acceptance criteria or new user interface behaviors:

  • Login as an admin user in Safari
  • Create an admin set collection and save
  • Select the Release and Visibility tab
  • See the placeholder text for the release dates fields
  • Select "Depositor must choose embargo"
  • Enter date as 10/23/2025 and save
  • See the date appears in the same format as it was entered

ezgif com-video-to-gif (4)

@samvera/hyrax-code-reviewers

@Dananji Dananji changed the title Fix release date issue in Safari [WIP] Fix release date issue in Safari Oct 7, 2019
@Dananji Dananji force-pushed the fix-safari-date-issue-4005 branch 2 times, most recently from f5a66d0 to cab3b80 Compare October 8, 2019 13:43
@Dananji Dananji changed the title [WIP] Fix release date issue in Safari Fix release date issue in Safari Oct 8, 2019
@Dananji Dananji marked this pull request as ready for review October 8, 2019 14:17
@Dananji
Copy link
Contributor Author

Dananji commented Oct 8, 2019

According to the coveralls report, code coverage is reduced in app directory which does not include assets and views directories and this PR contains changed files only in those 2. I'm not sure, whether I'm reading this correctly.
Anyone's help on this is appreciated. Thanks.
@samvera/hyrax-code-reviewers

@no-reply
Copy link
Member

we're okay to ignore coveralls. there's a proposal to remove it altogether since it does this constantly.

GitHub is setup to not require it, but still produces the scary red X.

Merge on this is just blocked by a review. Hoping someone with frontend expertise can take a peek at this.

cc: @adamjarling

Copy link
Contributor

@jrgriffiniii jrgriffiniii left a comment

Choose a reason for hiding this comment

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

This is fantastic, thank you very much for this @Dananji! A quick question: why was var used in place of let here for ES6? My assumption is that the former is preferred: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let#Temporal_dead_zone

adamjarling
adamjarling previously approved these changes Oct 30, 2019

var $form = $('form');
$form.on('submit', function() {
var data = $(this).serializeArray();
Copy link
Member

Choose a reason for hiding this comment

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

Agree with @jrgriffiniii.. for any es6/es7 JavaScript lets stick with either let or const variable declarations?

Copy link
Contributor Author

@Dananji Dananji Oct 30, 2019

Choose a reason for hiding this comment

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

Sure! I will make that change

@adamjarling
Copy link
Member

Code looks good to me.

@Dananji Dananji merged commit 58077c5 into master Oct 31, 2019
@Dananji Dananji deleted the fix-safari-date-issue-4005 branch October 31, 2019 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Safari date issues in admin set > release tab
4 participants