Skip to content

DatePicker: Show first selected date when DatePicker is nested #8382

Merged
henon merged 6 commits intoMudBlazor:devfrom
Anu6is:Fix-Wrapped-DatePicker-Selection
Mar 18, 2024
Merged

DatePicker: Show first selected date when DatePicker is nested #8382
henon merged 6 commits intoMudBlazor:devfrom
Anu6is:Fix-Wrapped-DatePicker-Selection

Conversation

@Anu6is
Copy link
Contributor

@Anu6is Anu6is commented Mar 16, 2024

Description

When the DatePicker is wrapped/nested in another component (example MudForm or MudGrid), the first selected date does not update the input box value.
It should be noted that the Date and Text properties are successfully updated, however it would appear to the user as thought nothing happened since <input> isn't set.

Resolves #5708

How Has This Been Tested?

Tested using the unit test viewer

Before
image

After
image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@github-actions github-actions bot added bug Unexpected behavior or functionality not working as intended PR: needs review labels Mar 16, 2024
@ScarletKuro
Copy link
Member

Very interesting that is requires Task.Run.
Does it work correctly both on BSS and WASM?

@Anu6is
Copy link
Contributor Author

Anu6is commented Mar 18, 2024

Very interesting that is requires Task.Run. Does it work correctly both on BSS and WASM?

It does, yes.

@Mr-Technician
Copy link
Member

Very interesting that is requires Task.Run. Does it work correctly both on BSS and WASM?

Can you provide justification for the usage of Task.Run? @Anu6is

@Anu6is
Copy link
Contributor Author

Anu6is commented Mar 18, 2024

Very interesting that is requires Task.Run. Does it work correctly both on BSS and WASM?

Can you provide justification for the usage of Task.Run? @Anu6is

I don't have any concrete justification. Just that when I was looking into it I couldn't see why it was behaving the way it did. Given that it was operating in an async context I wondered if there was some weird race condition occurring and just decided to test awaiting it.

Even after that worked I tried a few things without it, but it's the only thing that resolved the issue

@ScarletKuro
Copy link
Member

ScarletKuro commented Mar 18, 2024

Yeah, it could be. I see a lot of async void code. Maybe even somewhere exception happens, but it's swallowed because of that.
I don't know how MudForm works, but doesn't it start some timer work and maybe something is not on the UI thread and it deadlocks / throws exception. Maybe if someone looks on the parallel stack it would give an idea where does this SetDateAsync works on a threadpool, UI etc.

@henon
Copy link
Contributor

henon commented Mar 18, 2024

@ScarletKuro shall we merge this or do you think we need to investigate some more?

@ScarletKuro
Copy link
Member

I actually don't see that the added WrappedDatePickerTest.razor was used anywhere in the TestFixture with bUnit. Do I miss something?
If this works and there seems no visible side effects then we can merge.
But in future we should clearly refactor and make Clear -> Task ClearAsync, same with the Submit method.

@Anu6is
Copy link
Contributor Author

Anu6is commented Mar 18, 2024

Updated with a bUnit test.

But in future we should clearly refactor and make Clear -> Task ClearAsync, same with the Submit method.

Also, I second this ^

@henon henon merged commit 1df0d8d into MudBlazor:dev Mar 18, 2024
@henon
Copy link
Contributor

henon commented Mar 18, 2024

Thanks @Anu6is

@oretodd
Copy link

oretodd commented Apr 5, 2024

This is not fixed in v6.19.1.
I spent too much time trying to find a work-around, and ultimately failed.
If anyone has a work-around for it, I'd love to see it.

Interestingly, the problem only occurs for me if I tab into the MudDatePicker field, and then click to open the picker.
If I click the (unopened) field when it does not already have the focus, it works correctly, and the selected date shows.

@Anu6is Anu6is deleted the Fix-Wrapped-DatePicker-Selection branch April 12, 2024 12:39
biegehydra pushed a commit to biegehydra/MudBlazor that referenced this pull request Apr 26, 2024
…azor#8382)

* DatePicker: Show first selected date when nested

* Invoke

* add unit test
@Herbstwc
Copy link

Herbstwc commented May 8, 2024

This is not fixed in v6.19.1. I spent too much time trying to find a work-around, and ultimately failed. If anyone has a work-around for it, I'd love to see it.

Interestingly, the problem only occurs for me if I tab into the MudDatePicker field, and then click to open the picker. If I click the (unopened) field when it does not already have the focus, it works correctly, and the selected date shows.

Also not fixed for me.

My scenario is that the MudDatePicker is in a MudContainer, works fine on desktop but not on mobile then the selected date does not show after first selection.

My workaround is to simply set the date after it has changed.

<MudDatePicker Date="DobValue" DateChanged="DobChange" DateFormat="dd/MM/yyyy" Label="Date Of Birth" />

DateTime? DobValue { get; set; }

void DobChange(DateTime? newDate)
{
    DobValue = newDate;

    StateHasChanged();
}

@Anu6is
Copy link
Contributor Author

Anu6is commented May 8, 2024

@oretodd @Herbstwc one of you should create a new issue explaining the scenarios under which it does not work.

If I click the (unopened) field when it does not already have the focus, it works correctly, and the selected date shows.

This is what was fixed based on the discussion item #5708

  1. The problem only occurs for me if I tab into the MudDatePicker field, and then click to open the picker.
  2. Works fine on desktop but not on mobile then the selected date does not show after first selection.

Details such as these should be added to a new issue to be investigated.

@Ztomporo
Copy link

I am going to start digging into my issue. But I am trying to upgrade up to 6.21 before I make the jump to 7 This seems to be causing me issues where the initial value gets set then gets overwritten with a null value and removes it from the input field. If I find a solution or exact cause I will update here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants