Skip to content

MudAutocomplete: Fix search not being triggered when the text is cleared#9599

Merged
henon merged 4 commits intoMudBlazor:devfrom
vernou:9495-autocomplete-search
Aug 18, 2024
Merged

MudAutocomplete: Fix search not being triggered when the text is cleared#9599
henon merged 4 commits intoMudBlazor:devfrom
vernou:9495-autocomplete-search

Conversation

@vernou
Copy link
Member

@vernou vernou commented Aug 9, 2024

Description

This PR fixes #9495 :

When ResetValueOnEmptyText is enable and the text cleared, the search isn't called to refresh the drop down list.

How Has This Been Tested?

I added a test to simulate the case.

Type 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)
  • Documentation (fix or improvement to the website or code docs)

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 Aug 9, 2024
@vernou
Copy link
Member Author

vernou commented Aug 9, 2024

A hack in MudBaseInputprevents the bug in server mode. I don't know how execute the tests as WASM, so I temporally disable the hack to check the added test fail in GitHub action.

Next I will push the fix and finally restore the hack.

@vernou
Copy link
Member Author

vernou commented Aug 9, 2024

In the GitHub action, the added test ResetValueOnEmptyText_WhenTextCleared_ThenSetNullAndTriggerSearch fails. The test TextField_TextUpdateSuppression also fails, because the hack is removed.

After the fix is pushed, in the GitHub action the added test ResetValueOnEmptyText_WhenTextCleared_ThenSetNullAndTriggerSearch success. The test TextField_TextUpdateSuppression always fail.

Finally, after the hack was restored, all tests success in the GitHub action.

@codecov
Copy link

codecov bot commented Aug 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.55%. Comparing base (28bc599) to head (a279116).
Report is 434 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #9599      +/-   ##
==========================================
+ Coverage   89.82%   90.55%   +0.72%     
==========================================
  Files         412      405       -7     
  Lines       11878    12751     +873     
  Branches     2364     2469     +105     
==========================================
+ Hits        10670    11547     +877     
+ Misses        681      644      -37     
- Partials      527      560      +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ScarletKuro
Copy link
Member

ScarletKuro commented Aug 10, 2024

I don't know how to execute the tests as WASM

You can't do this in bUnit since it always executes in a standard .NET environment.

What we can do, and I believe this is the correct approach, is to implement something like this:

namespace MudBlazor.Utilities;

/// <summary>
/// Provides methods to determine whether the application is running on the client-side or server-side.
/// </summary>
[ExcludeFromCodeCoverage]
public class RuntimeLocation
{
    /// <summary>
    /// An instance of the platform detector used to check the current runtime environment.
    /// </summary>
    internal static readonly IPlatformDetector PlatformDetector = new BrowserPlatformDetector();

    /// <summary>
    /// Gets a value indicating whether the application is running on the client-side (in a browser).
    /// </summary>
    public static bool IsClientSide => PlatformDetector.IsBrowser();

    /// <summary>
    /// Gets a value indicating whether the application is running on the server-side.
    /// </summary>
    public static bool IsServerSide => !PlatformDetector.IsBrowser();

    /// <summary>
    /// Defines an interface for detecting the platform on which the code is running.
    /// </summary>
    internal interface IPlatformDetector
    {
        /// <summary>
        /// Determines whether the application is running in a browser environment.
        /// </summary>
        /// <returns><c>true</c> if the application is running in a browser; otherwise, <c>false</c>.</returns>
        bool IsBrowser();
    }

    /// <summary>
    /// Detects if the current platform is a browser.
    /// </summary>
    internal class BrowserPlatformDetector : IPlatformDetector
    {
        /// <inheritdoc />
        public bool IsBrowser() => OperatingSystem.IsBrowser();
    }
}

Now, you can create your own implementation of IPlatformDetector within MudBlazor.UnitTests (in the Mocks area) and create a dummy implementation that returns IsBrowser() => true. Then, you can temporarily replace RuntimeLocation.PlatformDetector in your tests.
Now we have a testable RuntimeLocation.

@henon
Copy link
Contributor

henon commented Aug 11, 2024

@vernou I sent a DM on Discord but you didn't respond, so I am pinging you here.

@vernou
Copy link
Member Author

vernou commented Aug 13, 2024

@vernou I sent a DM on Discord but you didn't respond, so I am pinging you here.

I answered.

@vernou vernou mentioned this pull request Aug 13, 2024
2 tasks
@vernou
Copy link
Member Author

vernou commented Aug 13, 2024

@ScarletKuro, I created the issue #9629 to improve it, so it can be done later.

@vernou
Copy link
Member Author

vernou commented Aug 16, 2024

@henon Can you review this PR?

@henon
Copy link
Contributor

henon commented Aug 16, 2024

OK, I'll do it this weekend, I promise.

@henon henon changed the title MudAutocomplete: Fix the search that isn't trigger when the text is cleared MudAutocomplete: Fix search not being triggered when the text is cleared Aug 18, 2024
@henon henon merged commit e279503 into MudBlazor:dev Aug 18, 2024
@henon
Copy link
Contributor

henon commented Aug 18, 2024

Thanks Cedric, you found a very good fix.

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.

Autocomplete with ResetValueOnEmptyText does not trigger search when field is cleared

4 participants