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

Add readonly attribute to date_field so that user can not fill it manually #3705

Conversation

tramuntanal
Copy link
Contributor

@tramuntanal tramuntanal commented Jun 22, 2018

🎩 What? Why?

On the public side, in a date_field field if the date is entered manually (without using the calendar widget) this is not copied to the value of the hidden input that is sent.
By making the visible text field readonly the user will be forced to use the calendar widget avoiding the problem.

This is a fast bugfix expected to be added to 0.12.1. Will work on a better solution for 0.13.0.

📌 Related Issues

📋 Subtasks

  • Add CHANGELOG entry
  • [-] Add documentation regarding the feature
  • [-] Add/modify seeds
  • [-] Add tests

📷 Screenshots (optional)

Description

@ghost ghost assigned tramuntanal Jun 22, 2018
@ghost ghost added the status: WIP label Jun 22, 2018
Copy link
Contributor

@isaacmg410 isaacmg410 left a comment

Choose a reason for hiding this comment

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

Please review my comments

CHANGELOG.md Outdated
@@ -66,6 +66,7 @@ Decidim::Organization.find_each { |organization| Decidim::System::CreateDefaultP

**Added**:

**decidim-core**: Add readonly attribute to date_fields so that the user is forced to use the datepicker. [#3705](https://github.com/CodiTramuntana/decidim/pull/3705)
Copy link
Contributor

Choose a reason for hiding this comment

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

the url is correct? should not be "https://github.com/decidim/decidim/pull/3705" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are totally right!

Copy link
Contributor

@isaacmg410 isaacmg410 left a comment

Choose a reason for hiding this comment

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

Looks fine to me!

@tramuntanal
Copy link
Contributor Author

The failing test is passing locally. May "unknown error: Chrome failed to start: exited abnormally" be a random test failure? can @decidim/lot-core re-execute it please?

@tramuntanal
Copy link
Contributor Author

It worked! Ready to review and merge @decidim/lot-core !

@oriolgual
Copy link
Contributor

This is a fast bugfix expected to be added to 0.12.1. Will work on a better solution for 0.13.0.

Can you elaborate on this @tramuntanal ?

@tramuntanal
Copy link
Contributor Author

Hi @oriolgual ,
There are many installations right now with this problem so I'm proposing a sub-optimal bugfix to be published asap, but then keep working with the issue.

My idea is to allow users to enter the date manually in their locale format and then, make controllers parse this strings to date before the parameter goes deeper into the business application logic.

@oriolgual oriolgual merged commit 3aa2a7c into 0.12-stable Jun 22, 2018
@oriolgual oriolgual deleted the fix/3574-date_field_doesnt_work_when_date_is_entered_manually-readonly branch June 22, 2018 14:17
juliesimon pushed a commit to OpenSourcePolitics/decidim that referenced this pull request Jul 9, 2018
…ually (decidim#3705)

* Add readonly attribute to date_field so that user can not fill it manually.

* Add Changelog entry.

* [FIX] URL for PR in changelog was incorrect.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants