FileUpload: Rewrite using ActivatorContent pattern

[igotinfected]

Description

Replaced ButtonTemplate with ActivatorContent. Anything within ActivatorContent now activates the file picker.

This allows us to use normal button tags (which can automatically be focused by keyboards) and trigger the OpenFilePicker action with a mouse click or a keyboard press (space, enter, ...).

Because of the rename (and the removal of FileUploadButtonTemplateContext) this is now a breaking change.

Resolves #5779

How Has This Been Tested?

Visually + unit tests.

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)

Basic example (with keyboard trigger)

keyboard-accessibility.mp4

Drag and Drop example

drag-and-drop.mp4

Documentation review

Reworded the first excerpt and the use of MudButtons:

Reworded and fixed styling on the Form Validation example:

Checklist

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

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 90.13%. Comparing base (28bc599) to head (d1383db).
Report is 108 commits behind head on dev.

Files	Patch %	Lines
...lazor/Components/FileUpload/MudFileUpload.razor.cs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #8694      +/-   ##
==========================================
+ Coverage   89.82%   90.13%   +0.30%     
==========================================
  Files         412      419       +7     
  Lines       11878    12195     +317     
  Branches     2364     2403      +39     
==========================================
+ Hits        10670    10992     +322     
+ Misses        681      662      -19     
- Partials      527      541      +14     

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

[ScarletKuro]

Oh, didn't meant to select "approve" yet.

so far I've only been able to do this by adding a JS function to .click() on the InputFile's input, open to suggestions if anyone knows how to do it without JS

Don't think so, at least definitely not without JS

[danielchalmers]

Did some research and agree it's not easy without JS. Not sure about the context command though; Why can't the parent handle the onclick so you don't have to add that handler yourself?

[igotinfected]

Did some research and agree it's not easy without JS. Not sure about the context command though; Why can't the parent handle the onclick so you don't have to add that handler yourself?

I think this would work if we limited the MudFileupload to a MudFileuploadButton, but today it's used in a way where the ButtonTemplate really is just a ChildContent. What I mean by that is that we already have complex Fileupload scenarios with multiple buttons (or clickable elements) within ButtonTemplate: https://mudblazor.com/components/fileupload#custom-scenario (the drag and drop example)

I don't have the offhand knowledge right now but I would imagine if we did it with an onclick handler on the parent users would have to play with stopPropagation and preventDefault to avoid other buttons being impacted?

My first implementation also played around with giving focus to the hidden input as a quick and dirty solution but that meant the actual upload button didn't get it's hover/focus effect, so I think this solution would also require additional CSS (probably just adding the same classes to the div as the MudButton).

[danielchalmers]

I'm confused as to why that example stuffed everything inside the button template instead of just placing it outside? I think the button template should be used purely for buttons with the expectation that a click effect will trigger the dialog.

I think @onclick should be fine as long as it's understood that's how it works and the above is addressed 🤷‍♀️

Yeah I had no luck with the input. I think what you have makes sense it's just that the button needs to be a, well, button 😄

[igotinfected]

I'm confused as to why that example stuffed everything inside the button template instead of just placing it outside? I think the button template should be used purely for buttons with the expectation that a click effect will trigger the dialog.

I think @onclick should be fine as long as it's understood that's how it works and the above is addressed 🤷‍♀️

Yeah I had no luck with the input. I think what you have makes sense it's just that the button needs to be a, well, button 😄

I just remembered I was the one that added the ClearAsync method to the ButtonTemplateContext to fix an issue with form validation: #7720

At the time there were talks about adding a delete button to MudFileupload for v7, might be the right moment to reopen that discussion (if only to redefine what MudFileupload should really be).

Essentially we need:

• a button to open the file picker
• some way to clear selected files
• validation to work as expected
• keyboard friendly usage for that file picker button

I have no implementation preference as long as those 4 points are met 👌

[henon]

@Mr-Technician if you can find some time to review this please do so.

[danielchalmers]

MudMenu's ActivatorContent is what I picture

[Mr-Technician]

I agree with @danielchalmers. The ActivatorContent route is the direction I would like to go if we are making a breaking change anyway, provided the drag and drop examples still work as expected.

At the time there were talks about adding a delete button to MudFileupload for v7, might be the right moment to reopen that discussion (if only to redefine what MudFileupload should really be).

Would a delete button behave any differently from calling ClearAsync in user-implemented button?

[igotinfected]

Would a delete button behave any differently from calling ClearAsync in user-implemented button?

Nope don't think it would -- I think I just have a preference for template context over component references in general. In this case I agree with @danielchalmers though in that MudFileUpload should just be a file picker button.

Should we also try to remove FileUploadButtonTemplateContext (I think we won't need it if we implement it the same way it is done in MudMenu, not 100% sure)?

[igotinfected]

Status report:

Made progress with the ActivatorContent approach yesterday, I have working standard button examples and a working drag and drop example (where just the drag and drop area is the ActivatorContent for MudFileUpload).

Will push the changes and clean up unit tests and docs over the weekend!

Edit: title and PR description will definitely need a change with these changes, to be updated once reviews are finalised.

[igotinfected]

Ready for re-review! ButtonTemplate is now ActivatorContent and FileUploadButtonTemplateContext is no longer necessary. All previous examples still work, including the drag and drop example.

Keyboard navigation to standard buttons now works since we no longer need the label + for solution, and users can theoretically use whatever they want within ActivatorContent.

This is now a breaking change due to the name change (users should be encouraged to switch away from <MudButton HtmlTag="label ... anyway) and because of the removal of the FileUploadButtonTemplateContext. ClearAsync and OpenFilePickerAsync are public methods that achieve the same purpose.

[henon]

@igotinfected Please add the renamed parameter to the illegal razor parameter detector. See #8771

[igotinfected]

@igotinfected Please add the renamed parameter to the illegal razor parameter detector. See #8771

Done! Simulated error here:

Great idea for the huge migration, thank you!

[Mr-Technician]

Thanks! @igotinfected

[henon]

Added to v7.0.0 Migration Guide #8447