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

Using DatePicker inside a PropertyColumn on a MudDataGrid, date does not update on selection. #8906

Closed
1 of 2 tasks
ibc-shane-kelly opened this issue May 7, 2024 · 13 comments
Labels
answered Topic has been answered

Comments

@ibc-shane-kelly
Copy link

Bug type

Component

Component name

MudDataGrid / MudDatePicker

What happened?

After updating from Version 6.17 to either 6.18 of 6.19.1, The DatePicker no longer sets its date on selection when embedded in a Property Column on a MudDataGrid.

Expected behavior

After Selecting a Date the displayed date is expected to update to reflect the selected date.

Reproduction link

https://try.mudblazor.com/snippet/macoaTYBIulQldPP

Reproduction steps

  1. Open the date picker,
  2. select a date
    3.The date does not update.

Relevant log output

No response

Version (bug)

6.18 and 6.19.1

Version (working)

6.17

What browsers are you seeing the problem on?

Firefox, Edge

On which operating systems are you experiencing the issue?

Windows

Pull Request

  • I would like to do a Pull Request

Code of Conduct

  • I agree to follow this project's Code of Conduct
@ScarletKuro
Copy link
Member

Hi.

After updating from Version 6.17 to either 6.18 of 6.19.1,

Are you sure in your statement?
I copy pasted your code, downgraded to 6.17.0 and it doesn't work, I downgraded to 6.2.3 and it doesn't work.]

But it works just fine when I have a normal class that holds the DateTime even with the current version.
https://try.mudblazor.com/snippet/QkcSapOLVIZXjWGP

@ScarletKuro
Copy link
Member

This is tricky to explain, but it's expected behavior.
In your snippet you directly declare List<DateTime?>, the issue arises because DateTime is a value type, this means that each time it's accessed a copy will be created, therefore when you are editing it you are not working anymore with the "original" DateTime. However in my snippet I wrap the DateTime within a class Item and therefore working with a reference type.
I hope you do understand the difference between a class and a struct.

@ScarletKuro ScarletKuro added the answered Topic has been answered label May 7, 2024
@ibc-shane-kelly
Copy link
Author

Thank you so much for getting back to me so soon.

Yes sorry my mistake I was trying to simplify the actual example and saw the behavior of not updating was the same and assumed I had done a good job.

I have updated my test code to more closely resemble the actual code I am having an issue with after the MudBlazor Update until I found the issue happening. Then I took most of it out again ensuring the issue still existed. I still may be doing something incorrect but it turns out after the update I am only having the trouble when utilizing the DatePicker Property TextChanged

Here is my updated test using your working example as a base.

https://try.mudblazor.com/snippet/caQSYJuCniMeLjum

Please let me know if I should be doing something different or if this is a bug.

Thanks again for your time.

Kind regards

Shane

@lanvo
Copy link

lanvo commented May 8, 2024

From your code sample, I think this is the reason. When this method get called StartDateChanged(Item? item), it still have the old value passed to it. So the data bind event of the grid happens after StartDateChanged.

@ScarletKuro
Copy link
Member

ScarletKuro commented May 8, 2024

Hi again.

Any reason you are not satisfied with the @bind-Date="context.Item.Value" without TextChanged? I would say this is the only correct way to do it.

From your code sample, I think this is the reason. When this method get called StartDateChanged(Item? item), it still have the old value passed to it. So the data bind event of the grid happens after StartDateChanged.

This is correct.
You expect that the DataGrid will mutate context.Item and you receive the modified one but DataGrid doesn't touch this context. It will somewhere internally deep clone it and modify and use if needed but it's not exposed anywhere(and this will actually happen using only the Form mode, in Cell mode DataGrid is not even aware how the data is being modified by custom EditTemplate).
For the TextChanged to work you'd need to do something like this:
https://try.mudblazor.com/snippet/mOweEzOCSuAZJMPA

I haven't yet tested if this indeed worked in previous versions, but I'd be surprised if it did and I actually don't recall any DataGrid changes for the EditTemplate, but I will check when I get time.

@ScarletKuro
Copy link
Member

ScarletKuro commented May 8, 2024

I haven't yet tested if this indeed worked in previous versions, but I'd be surprised if it did and I actually don't recall any datagrid changes for the EditTemplate, but I will check when I get time.

The only thing I can think of, is that earlier the bind-Date and TextChanged worked at same time, and the two way binding was modifying the Value and when StartDateChanged fires it was having than new value. Now the @bind-Date doesn't work when TextChanged is used. But TextChanged doesn't really make sense to me when you are using @bind-Date

@ScarletKuro
Copy link
Member

@henon @Anu6is this change #8382 "broke" it.
It's hard to say if I'm more leaning towards it to be a feature than a bug, since I'm still thinking that OP is using the code incorrectly.
It looks like my above assumption is correct:
Before the change the sequence would be: TextChanged -> DateChanged -> TextChanged -> DateChanged
After the change the sequence is: TextChanged -> TextChanged -> DateChanged -> DateChanged
Don't ask me why the events are firing twice, I guess it's another mublazor bug with the "call the eventhandler in the setter" and something is triggering it twice. But you still shouldn't rely on this sequence side effects on what fires first, that's why I probably wouldn't do anything with the #8382 but let me know what ya all think.

@Anu6is
Copy link
Contributor

Anu6is commented May 8, 2024

I think the component just needs some attention. Update to the new parameter state usage and see if any of the logic can be streamlined/simplified

@ibc-shane-kelly
Copy link
Author

Thanks for the Info, to give you an idea of why I am using TextChanged as well as @bind-Date. If the full code I actually use PickerClosed as well but that event is still working as before.

The @bindDate is bound to a date property of an object that is a part of a collection assigned to the grid. @bind-Date updates the value perfectly well, but I am looking for an event to have the modification of that date affect other dates in the collection to make user input easier.

For example in the actual code the object has a StartDate and an EndDate, when the user updates the start date depending on if they used a text change or a picker change we are catching that event and updating the equivalent finished date. With out the individual event for the specific date being changed it would be difficult to determine which date was changed in the larger object to know which end date to update.

Let me know if this doesn't make sense I can update an example further to illustrate if you like.

Kind regards

Shane

@ScarletKuro
Copy link
Member

ScarletKuro commented May 9, 2024

With out the individual event for the specific date being changed it would be difficult to determine which date was changed

Do I understand correct that you just need an event when the binding is updated?
Use:

<MudDatePicker @bind-Date="date" @bind-Date:after="DateUpdated" />

Unfortunately the :after doesn't work in trymudblazor, but it should in project with .net7 and higher.

@ibc-shane-kelly
Copy link
Author

Great, thanks! you learn something new every day.

That is working, I'm going to need to work on my Error notifications on the DatePicker now as they seem to now appear in times that they did using the old method but the date is updating and the corresponding date is being updated correctly as well.

Thank you!

@amaxtesting
Copy link

I use @key to refresh DatePicker

@ScarletKuro
Copy link
Member

Hi, I will close this issue since we have helped as much as we could.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered Topic has been answered
Projects
None yet
Development

No branches or pull requests

5 participants