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

Issue with using OnSelect event with FileManager #3010

Closed
vnetonline opened this issue Jul 12, 2023 · 8 comments
Closed

Issue with using OnSelect event with FileManager #3010

vnetonline opened this issue Jul 12, 2023 · 8 comments

Comments

@vnetonline
Copy link
Contributor

I am trying to use the FileManager component so that iI can select an Image (File) it should populate the Name Field

Here is the code of the FileManager Component

<div class="col-sm-9">
    <FileManager FileId="@_slideFileId" Filter="@Constants.ImageFiles" 
        ShowFolders="false" ShowFiles="true" UploadMultiple="false"  
        Folder="Public" @ref="fileManager" OnSelect="PopulateFileName" />
</div>

  private async Task PopulateFileName(int fileId)
  {

      if (fileId != -1 && _slideFileId != fileId)
      {
          _slideFile = await FileService.GetFileAsync(fileId);
          _name = _slideFile.Name;
      }

  }

Here is what I see when I edit an item - image flickers because FileId becomes -1 after the PopulateFileName is called because as per Blazor it automatically calls StateHasChanged()

screen2gif-filemanager-onselect-behaviour-edit

Here is what I see when I add an item - no image preview is displayed but the FileName is populated

screen2gif-filemanager-onselect-behaviour-add

@vnetonline
Copy link
Contributor Author

@leigh-pointer can you verify for me if this is an issue?

@vnetonline
Copy link
Contributor Author

@sbwalker & @leigh-pointer

I definitely think there is a problem with the OnSelect Event callback, from what I have read that EventCallback call StateHasChanged automatically which results in The Parent Component and its child component rerendering which results in the FileId becoming -1 and the FileChanged firing again causing a loop.

Have eaither of you dealth with this issue?

vnetonline added a commit to vnetonline/oqtane.framework that referenced this issue Jul 14, 2023
@vnetonline
Copy link
Contributor Author

The code in my razor looks like this

private void PopulateFileName(int fileId)
{

    if (fileId != -1 && _slideFileId != fileId)
    {
        _slideFile = fileManager.GetFile();
        Name = _slideFile.Name;
        _slideFileId = fileId;
        StateHasChanged();
    }

}

screen2gif-filemanager-onselect-behaviour-fixed

@vnetonline
Copy link
Contributor Author

Refer to this thread

dotnet/aspnetcore#31549

which has some workarounds hence I changed OnSelect to a Action from EventCallback

@sbwalker
Copy link
Member

sbwalker commented Jul 14, 2023

@vnetonline I think the issue is that the FileManager was recently modified to utilize the OnParametersSet() method rather than the OnInitialized() method. This was done so that it is possible to change the values of parameters passed from your own component to the FileManager and have the FileManager respond to those changes. However there is a reference to OnSelect in this method:

       if (FileId != -1)
        {
            File file = await FileService.GetFileAsync(FileId);
            if (file != null)
            {
                FolderId = file.FolderId;
>>                await OnSelect.InvokeAsync(FileId);
            }
            else
            {
                FileId = -1; // file does not exist
                _message = "FileId " + FileId.ToString() + "Does Not Exist";
                _messagetype = MessageType.Error;
            }
        }

I am not sure why it is necessary to call OnSelect in this method as the FileId is already known by the caller (it was passed as a parameter). So I think the OnSelect call should be removed. Either that, or the call to OnSelect should be move to OnInitialized() where it will only execute once - however I don't think it should execute at all.

@sbwalker
Copy link
Member

sbwalker commented Jul 14, 2023

@vnetonline the other problem with your code is that when OnSelect is fired, you are not setting the variable in your calling component which holds the FileId parameter state. You specify FileId="@_slideFileId" as a parameter which means that _slideFileId holds the state and then when OnSelect is fired you are not assigning _slideFileId to the new value of fileid. This means that when FileManager is re-rendered, _slideFileId will still have the original value which was set on the initial render... which is -1. Your code should be modified to:

<div class="col-sm-9">
    <FileManager FileId="@_slideFileId" Filter="@Constants.ImageFiles" 
        ShowFolders="false" ShowFiles="true" UploadMultiple="false"  
        Folder="Public" @ref="fileManager" OnSelect="PopulateFileName" />
</div>

  private async Task PopulateFileName(int fileId)
  {

      if (fileId != -1 && _slideFileId != fileId)
      {
          _slideFileId = fileId;
          _slideFile = await FileService.GetFileAsync(_slideFileId);
          _name = _slideFile.Name;
      }
  }

@vnetonline
Copy link
Contributor Author

You are right @sbwalker the looping was being caused by await OnSelect.InvokeAsync(FileId); in the OnParametersSetAsync
I have fixed up the pull request and moved it to protected override async Task OnInitializedAsync()

    private async Task FileChanged(ChangeEventArgs e)
    {
        _message = string.Empty;
        FileId = int.Parse((string)e.Value);
        if (FileId != -1)
        {
            await SetImage();
            await OnSelect.InvokeAsync(FileId);
        }

    }

I have left await SetImage(); before the await OnSelect.InvokeAsync(FileId); and removed StateHasChanged in the above code block

  1. So that the file is set before the Event is called so that the Event Consumer can utilise the file rather than calling FileService and getting the file again.
  2. I Have removed the StateHasChanged because the EventCallBack automatically calls StateHasChanged calling it two times was making the image Flicker

Thanks for your help @sbwalker first time making a module and fairly new to Blazor learning something every day.

@vnetonline
Copy link
Contributor Author

@sbwalked fixed this in #3024

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

No branches or pull requests

2 participants