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

Change behavior when changing dates in a datepicker group to be more intuitive. #2583

Merged
merged 5 commits into from
Nov 13, 2024

Conversation

drgrice1
Copy link
Member

Previously when any of the dates were changed, the open, reduced scoring, close, and answer dates were ensured to be in the correct order by always changing the input later in the last to a later date if needed.

This instead acts on the input that was modified. That input value is not changed from what the user just changed it to. Any earlier inputs in the list are set to the new date in the changed input if they are later than that date, and any later inputs in the list are set to the new date in the changed input if they are earlier than that date.

This is to address issue #2581.

@drgrice1 drgrice1 force-pushed the datepicker-group-update-tweak branch from 8391a24 to 13afbcf Compare September 27, 2024 16:11
@somiaj
Copy link
Contributor

somiaj commented Sep 28, 2024

Overall this is what I was asking for in #2581, though I may have not been clear enough there. I just wanted user input to always be honored and the other dates updated based on that.

In testing, this doesn't seem to be fully working for me, as the open date is never being adjusted under any of my tests when it should. My tests are to set all the dates (open, reduced, close, answer) to be the same. Then when I set the answer, closed, or reduced date to be an earlier date, the open date isn't updated (all the other dates are updated appropriately though).

@drgrice1
Copy link
Member Author

The open date updates when I test it. 😜

Copy link
Contributor

@somiaj somiaj left a comment

Choose a reason for hiding this comment

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

Works fine now, I must have been hallucinating.

@drgrice1
Copy link
Member Author

You did see that I added another commit after you made the comment?

@somiaj
Copy link
Contributor

somiaj commented Sep 28, 2024

Yea, I saw the fix, I tried to figure out why the open date was being ignored, but missed the loop index needed changed.

@drgrice1 drgrice1 force-pushed the datepicker-group-update-tweak branch from d6c0d02 to 267a826 Compare October 3, 2024 01:48
@somiaj
Copy link
Contributor

somiaj commented Oct 10, 2024

I seeing the following behavior with the date picker and setting the dates for individual user(s). Go override the dates for a single user, set all the dates to the same date in the future (I'm using a full year). Then delete the 'close' date. When this happens the reduced scoring and open date are reset, but they aren't rest to a valid date corresponding to the set close date. Here is a screen shot.

image

To me the dates should have reverted to 9/30/24, 12:00 PM (the set close date), but for some reason 10/2/24, 12:00 AM gets used.

@somiaj
Copy link
Contributor

somiaj commented Oct 10, 2024

Note, setting all the dates to some date over a year in the past, and deleting the open date, does reset the reduced, close, and answer date to equal the open date of the set, so I wasn't able to reproduce any issue with updating the dates in the other direction.

@drgrice1
Copy link
Member Author

Are you testing with this pull request? I just tested with this pull request and used the exact dates shown in your screen shot, and I don't see that. When I delete the close date, the open and reduced scoring dates are then set to 9/30/24, 12:00 PM.

@somiaj
Copy link
Contributor

somiaj commented Oct 10, 2024

Yes, I'm testing with this PR. Wonder what other settings in my set could be causing the problems.

@somiaj
Copy link
Contributor

somiaj commented Oct 10, 2024

Oh, I think I figured it out. I have previously overridden the close/answer date for this set/user to be 10/2/24, 12:00 AM, so that is where it is getting the date from.

@drgrice1 drgrice1 force-pushed the datepicker-group-update-tweak branch from c003744 to 28557d8 Compare October 18, 2024 15:09
@drgrice1 drgrice1 force-pushed the datepicker-group-update-tweak branch from 28557d8 to dd069f7 Compare October 29, 2024 19:26
@drgrice1 drgrice1 force-pushed the datepicker-group-update-tweak branch from dd069f7 to 9a725b0 Compare November 11, 2024 21:29
@drgrice1
Copy link
Member Author

@somiaj: I pushed a change that should address the issue that you saw. Basically, instead of falling back to the user's previous override when a date is deleted, it falls back to the course's date.

@drgrice1 drgrice1 force-pushed the datepicker-group-update-tweak branch 2 times, most recently from aabf37a to 43d34a7 Compare November 12, 2024 22:00
…intuitive.

Previously when any of the dates were changed, the open, reduced
scoring, close, and answer dates were ensured to be in the correct order
by always changing the input later in the last to a later date if
needed.

This instead acts on the input that was modified.  That input value is
not changed from what the user just changed it to.  Any earlier inputs
in the list are set to the new date in the changed input if they are
later than that date, and any later inputs in the list are set to the
new date in the changed input if they are earlier than that date.

This is to address issue openwebwork#2581.
Previously the open date was never acted on, and so the for loop started
at one which skipped the open date.  Now it needs to be acted on.
When using the "Today" or "Now" buttons, the "changed" class is not
being added to the input.  When the handlers for those buttons are
called, the "change" event needs to be triggered so that this happens.
@drgrice1 drgrice1 force-pushed the datepicker-group-update-tweak branch from 43d34a7 to fc659e1 Compare November 12, 2024 22:05
@Alex-Jordan
Copy link
Contributor

Alex-Jordan commented Nov 13, 2024

Tested and I get the desired behavior.

I'm not entirely following the finer point that was discovered if there is a previous date override. I will approve this and invite @somiaj to merge it if that issue is also resolved.

@somiaj
Copy link
Contributor

somiaj commented Nov 13, 2024

Great, this works well in this case now. Thanks for looking into that last minor issue.

@somiaj somiaj merged commit edbf4e2 into openwebwork:develop Nov 13, 2024
2 checks passed
@drgrice1 drgrice1 deleted the datepicker-group-update-tweak branch November 13, 2024 11:36
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.

3 participants