Skip to content

Add support for new dialog form method in FormMethod enum #44499

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

Closed
1 task done
Ducki opened this issue Oct 12, 2022 · 10 comments · Fixed by #44500
Closed
1 task done

Add support for new dialog form method in FormMethod enum #44499

Ducki opened this issue Oct 12, 2022 · 10 comments · Fixed by #44500
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue

Comments

@Ducki
Copy link
Contributor

Ducki commented Oct 12, 2022

Is there an existing issue for this?

  • 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 <dialog> 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
Copy link
Contributor Author

Ducki commented Oct 12, 2022

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

@TanayParikh TanayParikh added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Oct 12, 2022
@javiercn
Copy link
Member

@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
Copy link
Member

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 javiercn added feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views help wanted Up for grabs. We would accept a PR to help resolve this issue good first issue Good for newcomers. labels Oct 13, 2022
@javiercn javiercn added this to the .NET 8 Planning milestone Oct 13, 2022
@ghost
Copy link

ghost commented Oct 13, 2022

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.

@Ducki
Copy link
Contributor Author

Ducki commented Oct 13, 2022

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.

@javiercn
Copy link
Member

@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

@Ducki
Copy link
Contributor Author

Ducki commented Oct 15, 2022

Never mind, I got it in now :)

@ghost
Copy link

ghost commented Oct 18, 2022

@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.

@javiercn
Copy link
Member

@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
Copy link
Member

@Ducki thanks for the contribution!

@ghost ghost locked as resolved and limited conversation to collaborators Nov 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates feature-mvc-razor-views Features related to the Razor view engine for Razor pages and MVC views good first issue Good for newcomers. help wanted Up for grabs. We would accept a PR to help resolve this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants