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

User needs to be able to submit a date #72

Closed
ebdavison opened this issue Feb 4, 2020 · 5 comments · Fixed by #88
Closed

User needs to be able to submit a date #72

ebdavison opened this issue Feb 4, 2020 · 5 comments · Fixed by #88
Labels
enhancement This is a feature not a bug

Comments

@ebdavison
Copy link

Date fields are needed so format validation can be performed rather than using short answer (single-line) and deal with various date formats.

@Pierre-Sassoulas Pierre-Sassoulas added the enhancement This is a feature not a bug label Feb 4, 2020
@saschalalala
Copy link

I followed exactly the same approach I described at #71 (comment), just using date instead of float of course. However, the rendered input field is of type text. I looked up the django templates that are used by DateFields and it turns out, that they don't use <input type="date">, see https://docs.djangoproject.com/en/3.0/ref/forms/widgets/#dateinput (I think, because browser support/handling is still quite bad).

However, for this project, this problem leads to the question how to handle date and datetime input fields:

  • Is this something that needs to be addressed via js?
  • Is jquery the way to go inside this project and if yes would we add new jquery plugins?
  • Or would it make sense to drop jquery in favor of ES6 (and a bundler/transpiler)?

I don't know yet, how javascript inside this project is used and structured and I think, this ticket isn't the place to discuss this, but I wanted to start the discussion here because the date field is somewhat like the initial problem for these thoughts.

@Pierre-Sassoulas
Copy link
Owner

I think that we do not have custom javascript, we're using what is in bootstrap (so jquery). Also, not a lot of it, nor anything complicated. It would make sense to drop it if it's stretched too far by a feature. To be fair my knowledge is not up to date on the subject, but the migration to another framework should be easy because there aren't that much different forms to render (6? 8 with date and float ?). I would gladly merge a full migration that replaces bootstrap/jquery, especially if you add vue.js for #85 .

@saschalalala
Copy link

I searched the templates for <script> tags and I think, you aren't using any javascript at all at the moment, is that possible? Is there a specific reason then for these 3 script includes? https://github.com/Pierre-Sassoulas/django-survey/blob/master/survey/templates/survey/base.html#L23-L25

However, I think, not having too much javascript inside the surveys themselves is a very good idea, and there is no need for a migration. At the moment, it would (if I didn't miss anything) be very easy to drop the existing javascripts and replace them with a single bundled javascript file.

In order to use third party applications like e.g. flatpickr (https://flatpickr.js.org/) I would suggest using a module bundler like parcel (https://parceljs.org/) or not using a bundler at all and serving javascript extensions from the static dir manually, because setting up webpack for a project without many javascript dependencies seems a total overkill to me.

Regarding your comment about #85, I would consider them two different topics. In my opinion, writing a visualisation page as an API consuming frontend application in Vue does not mean that Vue is the right tool for the surveys themselves.

@Pierre-Sassoulas
Copy link
Owner

Is there a specific reason then for these 3 script includes?

They are necessaries because some of bootstrap's CSS classes that we use rely on it. I could be wrong, but it's possible that the only thing for which it's used is the dropdown for survey parts.

So yeah I pretty much agree with :

setting up webpack for a project without many javascript dependencies seems a total overkill to me.

It was included in a project that used boostrap/jquery where it made sense at the time. I don't like changing things in the javascript because those interactions are not tested automatically and I don't use the project currently so bugs could go undetected. Do not hesitate to make things saner though :)

@saschalalala
Copy link

Ah, I found some of them in one of the test surveys

image

Thanks, I didn't know about that functionality yet. Well, then it makes total sense to keep these files and extend them with flatpickr. I'll have a look about how to integrate it without much hassle. Forget everything I wrote about parcel then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is a feature not a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants