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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Please consider adding XML documentation for Fluent razor components #134

Closed
ShreyasJejurkar opened this issue Nov 21, 2021 · 13 comments
Labels
community:good-first-issue Good issues for first time contributors community:request Issues specifically reported by a member of the community. docs:api Additions, improvements, or fixes to API docs status:planned Work is planned

Comments

@ShreyasJejurkar
Copy link
Contributor

馃檵 Documentation Request

Please consider adding XML documentation for C# blazor component API.
Consider the following scenario, how come newcomers know what needs to be used for rendering anchor tags, as they see two components here.

image

If we compare this with inbuilt blazor components, at least we get some component information while typing itself.

image

馃拋 Possible Solution

We should consider adding XML documentation for these components and as well as their individual properties, as these comments can be inferred by Visual Studio and Visual Studio Code and provide a great experience for developers!

I am ready to help with this task to get it complete馃槄. But filling issue first so that we can track progress and I can know other people's thoughts on it if I miss anything here!

@EisenbergEffect EisenbergEffect added area:fast-blazor community:good-first-issue Good issues for first time contributors community:request Issues specifically reported by a member of the community. docs:api Additions, improvements, or fixes to API docs status:planned Work is planned labels Nov 29, 2021
@EisenbergEffect
Copy link
Contributor

This sounds like a a great improvement. We'd love to have this as a community contribution 馃槃

@ShreyasJejurkar
Copy link
Contributor Author

@EisenbergEffect Yes.
Can I know the place where we keep docs for FAST UI components so that I will try to match docs with their docs about each property and component so that it will be consistent for users!

@vnbaaij
Copy link
Collaborator

vnbaaij commented Nov 30, 2021

@ShreyasJejurkar
The official docs are here: https://docs.microsoft.com/en-us/fluent-ui/web-components/
The Fluent UI Web Components source is here: https://github.com/microsoft/fluentui/tree/master/packages/web-components. I would use the description fields from the xxx.vscode.definition.json files found in the web-components/src folder

@ShreyasJejurkar
Copy link
Contributor Author

Thanks, @vnbaaij. I will start working on this one-by-one component this upcoming weekend.

@vnbaaij
Copy link
Collaborator

vnbaaij commented Nov 30, 2021

@ShreyasJejurkar, @EisenbergEffect Maybe we should use this opportunity to also move the c# code out of the .razor files and into .razor.cs files. We can leverage new VS2022 functionality to do this and then manually remove unnecessary using and use file scope namspace.
Once this is done, Shrejas can use the 'insert ///' to let VS insert summary blocks automatically. Makes adding the comments a lot easier.

@ShreyasJejurkar
Copy link
Contributor Author

@vnbaaij Hmm. Yeh. That would be a good idea to have a separate file for code-behind .razor.cs. Right now the code is small so it's ok, but going forward it can be huge so better to go off with code-behind.
Am totally ok with that approach. 馃憤

@EisenbergEffect
Copy link
Contributor

@vnbaaij In order to minimize conflicts, how do we want to order the PRs? Should we get the namespace/using PR in first, then the update to .razor.cs and then the update to the doc comments? I can probably get the namespace PR merged today. Thoughts?

@ShreyasJejurkar
Copy link
Contributor Author

@EisenbergEffect Yup.

@vnbaaij 's namespace PR - > moving it to razor.cs - > xml docs!

@vnbaaij
Copy link
Collaborator

vnbaaij commented Nov 30, 2021

@EisenbergEffect If you canget #126 in first, I'll post another PR to move the code to separate files, update usings, etc in a bit (working on that rn)

@EisenbergEffect
Copy link
Contributor

#126 is now merged 馃槃 (I really like the new using and namespace features.)

@ShreyasJejurkar
Copy link
Contributor Author

@EisenbergEffect yup, that feature is quite good one!

@vnbaaij
Copy link
Collaborator

vnbaaij commented Nov 30, 2021

@EisenbergEffect #138 is ready for merging the changes mentioned above

@vnbaaij vnbaaij mentioned this issue May 17, 2022
6 tasks
@vnbaaij
Copy link
Collaborator

vnbaaij commented May 17, 2022

Closed with #188

@vnbaaij vnbaaij closed this as completed May 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community:good-first-issue Good issues for first time contributors community:request Issues specifically reported by a member of the community. docs:api Additions, improvements, or fixes to API docs status:planned Work is planned
Projects
None yet
Development

No branches or pull requests

3 participants