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

Changed datepicker for flatpicker #2790

Merged
merged 1 commit into from
Nov 6, 2024

Conversation

rjlanari
Copy link
Contributor

@rjlanari rjlanari commented Sep 30, 2024

*Replace the old datepicker with flatpickr and connect the flatpickr instances (if there are multiple ones, e.g. for module phases).

  • PR name contains story or task reference
  • Documentation (docs and inline)
  • Tests (including n+1 and django_assert_num_queries where applicable)
  • Changelog

@rjlanari rjlanari requested review from goapunk and m4ra September 30, 2024 11:03
requirements/dev.txt Outdated Show resolved Hide resolved
@rjlanari rjlanari requested a review from m4ra October 1, 2024 13:49
package.json Outdated Show resolved Hide resolved
@m4ra
Copy link
Contributor

m4ra commented Oct 1, 2024

@rjlanari @goapunk the flatpicker in mB works as dropdown, while the one here is similar to datepicker.
from mB dev:
image

from this a+ PR:
image

@rjlanari
Copy link
Contributor Author

rjlanari commented Oct 2, 2024

@rjlanari @goapunk the flatpicker in mB works as dropdown, while the one here is similar to datepicker. from mB dev: image

from this a+ PR: image

Hello @mara, why can it be that when I test it locally I get it as dropdown?

Screenshot 2024-10-02 at 11 17 23

@m4ra
Copy link
Contributor

m4ra commented Oct 7, 2024

@rjlanari you are right! I didn't rebuild the npm. Works as expected now, I will go ahead and merge. Thank you

@m4ra m4ra self-requested a review October 7, 2024 16:48
Copy link
Contributor

@m4ra m4ra left a comment

Choose a reason for hiding this comment

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

Nice!

@m4ra
Copy link
Contributor

m4ra commented Oct 7, 2024

@rjlanari can you run a rebase with main from your branch?

git fetch
git checkout your-branch
git rebase main

if any conflicts that you cannot resolve, let me know in mattermorst.
once done with rebase force push again.

@m4ra m4ra force-pushed the rl-2024-09-change-datepicker-to-flatpicker branch from 453833f to 218c616 Compare October 9, 2024 11:03
@rjlanari rjlanari requested a review from hom3mad3 October 9, 2024 12:55
$inputs.addClass('form-control')
function initDatePicker () {
const datepickers = document.querySelectorAll('.datepicker')
const format = django.get_format('DATE_INPUT_FORMATS')[0].replaceAll('%', '')
Copy link
Contributor

Choose a reason for hiding this comment

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

we could add a fallback date format here const format = django.get_format ? django.get_format('DATE_INPUT_FORMATS')[0].replaceAll('%', '') : 'Y-m-d';


$inputs.addClass('form-control')
function initDatePicker () {
Copy link
Contributor

@hom3mad3 hom3mad3 Oct 10, 2024

Choose a reason for hiding this comment

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

I would suggest splitting into smaller, well-named chunks and then call them inside the function. It will make the code easier to read and maintain and we can keep the main initializer nice and clean.

Copy link
Contributor

@goapunk goapunk Oct 15, 2024

Choose a reason for hiding this comment

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

not sure if that's something @rjlanari wants to take over as the focus was more on django, what do you think @rjlanari ? Otherwise maybe you can work on that @hom3mad3 if you have an idea in mind? If we change it we should port it over to mb (or if both files are identical we could even consider moving it into a4)

@goapunk goapunk force-pushed the rl-2024-09-change-datepicker-to-flatpicker branch from 218c616 to 1fcc0e0 Compare November 6, 2024 10:02
@goapunk goapunk enabled auto-merge (rebase) November 6, 2024 10:02
@goapunk goapunk merged commit 2ca3b26 into main Nov 6, 2024
2 checks passed
@goapunk goapunk deleted the rl-2024-09-change-datepicker-to-flatpicker branch November 6, 2024 10:10
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.

4 participants