aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

Add support for new dialog form method in FormMethod enum

Open Ducki opened this issue 3 years ago • 7 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Is your feature request related to a problem? Please describe the problem.

Currently, the FormMethod enum only contains the GET and POST form methods.

However, the new

element also introduced a new form method dialog (its use case is to close the dialog).

Currently, IDEs are showing a squiggle when using the

tag helper with this form method, probably because they rely on the content of this enum for that attribute.

Describe the solution you'd like

Add Dialog to FormMethod.

Additional context

No response

Ducki avatar Oct 12 '22 16:10 Ducki

If it helps, I created a PR for that change.

Ducki avatar Oct 12 '22 16:10 Ducki

@Ducki thanks for contacting us.

I think we will accept a contribution for this, although it seems that the public API baseline needs to be updated in the PR. If you open the solution in Visual Studio, there should be an analyzer that helps you fix the issue.

javiercn avatar Oct 13 '22 09:10 javiercn

For triage, support for <Dialog> is good in the latest browsers https://caniuse.com/?search=dialog (IE being the only one that does not support it, but that is expected).

It will be better if we can have confirmation that this is part of the actual standard and not just the living standard before we merge, as things tend to change there.

javiercn avatar Oct 13 '22 09:10 javiercn

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

ghost avatar Oct 13 '22 09:10 ghost

If you open the solution in Visual Studio, there should be an analyzer that helps you fix the issue.

I have tried for literally hours to get this single line in the Public API file.

Since I'm on a Mac, I tried running VS in a VM, which is practically with the ASP.NET project because it just chokes with errors and crashes (after trying to index and analyze for an hour (!)). Codespaces similar. Running the dotnet format analyzers cli tool is throwing errors as well and not finding some obscure files.

I'm really sorry. I really want to contribute, but it is borderline impossible to do so outside Windows/Visual Studio.

Ducki avatar Oct 13 '22 12:10 Ducki

@Ducki I am really sorry you run into issues with this. I'll try to get someone on the team to fix it and get it merged.

From what I understand this can be fixed running dotnet format in the project, but seems like we do not have this correctly documented. We have a separate issue tracking this here

javiercn avatar Oct 13 '22 13:10 javiercn

Never mind, I got it in now :)

Ducki avatar Oct 15 '22 12:10 Ducki

@Ducki @javiercn I don't mean to come in here, I was looking at good first issues this one seemed like an appropriate one but it seems it has been taken care of. Is the addition to Public API text file a way to track changes that have not been released into production directly to the users? I see those files in almost every single individual project.

ghost avatar Oct 18 '22 03:10 ghost

@WingZer0123 we track public API changes across versions using a set of Roslyn analyzers. You can see the details here

That way we know explicitly when we are removing APIs and can have a discussion about it.

javiercn avatar Oct 18 '22 08:10 javiercn

@Ducki thanks for the contribution!

javiercn avatar Oct 18 '22 08:10 javiercn