Megabit / Blazorise

Blazorise is a component library built on top of Blazor with support for CSS frameworks like Bootstrap, Tailwind, Bulma, AntDesign, and Material.
https://blazorise.com/
Other
3.29k stars 533 forks source link

Markdown image upload not working as expected in Blazor WASM #4299

Closed PatrickBig closed 1 year ago

PatrickBig commented 1 year ago

Describe the bug I'm attempting to update the File.UploadUrl property in the Blazorise.Markdown.Markdown component so it can render the image in the editor.

In my environment I cannot get the URL from ImageUploadChanged to ImageUploadEnded to set the Url.

To Reproduce

Environment:

Steps to reproduce the behavior:

Component

<Markdown 
    @bind-Value="@_input.Content"
    Autofocus="true" 
    ImageMaxSize="2097152"
    ImageUploadChanged="@OnImageUploadChanged"
    ImageUploadStarted="@OnImageUploadStarted"
    ImageUploadProgressed="@OnImageUploadProgressed"
    ImageUploadEnded="@OnImageUploadEnded"/>

Code behind file

private string? filePath;

[Parameter]
public string? ArticleId { get; set; }

[Inject]
private INewsManagerClient NewsManagerClient { get; set; }

async Task OnImageUploadChanged(FileChangedEventArgs e)
{

    try
    {
        foreach (var file in e.Files)
        {
            var response = await NewsManagerClient.AddNewsArticleAttachmentAsync(ArticleId, file);

            file.UploadUrl = response;

            _imageUploads[file.Name].Uri = response;
            filePath = response;

            file.Status = FileEntryStatus.Uploaded;
        }
    }
    catch (Exception exc)
    {
        Console.WriteLine(exc.Message);
    }
    finally
    {
        this.StateHasChanged();
    }
}

Task OnImageUploadEnded(FileEndedEventArgs e)
{
    if (e.Success)
    {
        e.File.UploadUrl = filePath; // filePath is always null here
    }
    else
    {
        e.File.UploadUrl = string.Empty;
    }

    return Task.CompletedTask;
}

Expected behavior I would expect one of two things:

  1. The IFileEntry would get it's update from being set after the upload completes when using the AddNewsArticleAttachmentAsync method. It's not. It's always null in OnImageUploadEnded
  2. The private field filePath would have the update. It's always null in OnImageUploadEnded

Additional context I have confirmed that the upload is indeed happening, I can see the files in my blob storage. I can also see my client is returning a valid URL to those resources as well.

David-Moreira commented 1 year ago

Hello,

the path is null because, OnImageUploadEnded occurs before the filePath = response; line since the image was already uploaded. Now, I see this as a valid use case. Having an api return an url and only then can you set the file path.

@stsrki I still do not know what changes we would need to do, but do you agree?


Also found another issue, once I commented out OnImageUploadedEnded, it throws an Exception. This should not happen. image image

And even removing the OnImageUploadedEnded callback from the Markdown causes the same Exception

stsrki commented 1 year ago

I still do not know what changes we would need to do, but do you agree?

I'm not 100% sure but it seems that we don't check for null on the JS side. That, or easymde is broken.

David-Moreira commented 1 year ago

Hello @stsrki it's two different things. 1— Allow to set file upload url after upload was completed. I.e receive url from api upload call. 2— 'null' exception

David-Moreira commented 1 year ago

@stsrki agree with 1? For number 2 I believe we can handle it ourselves.

stsrki commented 1 year ago

Well, try to do both. But I', not sure if 1 will be possible. I remember some limitation was there, not sure.

David-Moreira commented 1 year ago

Well, try to do both. But I', not sure if 1 will be possible. I remember some limitation was there, not sure.

Ok, will take a look but it's a pretty damn valid use case haha. I remember that actually we hit this road block when implementing on support forum, but since we could "calculate" the path before hand, there was no issue.