Docs: Document Admin conventions and relocate admin theme guides#19195
Conversation
|
Thank you for submitting your first pull request, awesome! 🚀 If you haven't already, please take a moment to review our contribution guide. This guide provides helpful information to ensure your contribution aligns with our standards. A core team member will review your pull request. If you like Orchard Core, please star our repo and join our community channels. |
|
Accept CLA |
|
@dotnet-policy-service agree |
|
Hi, I see the build failed due to some broken links after relocation. I'm currently away from my computer and will fix them as soon as I get back. Thanks for your patience! |
Removed 'Create a custom admin theme' entry and added new admin theme references.
|
You try the docs locally mkdocs serve -a 127.0.0.1:9999 |
Thanks for the suggestion! I will try running mkdocs serve locally to verify the links and fix the remaining warning. I'll update the PR as soon as it's verified. |
|
All checks have passed! I've fixed the relative link issues and verified them locally using mkdocs serve. Ready for another review. Thanks! |
Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
|
I've applied the suggestions. Ready for another review. Thanks! @hishamco @MikeAlhayek |
| - Placements: reference/modules/Placements/README.md | ||
| - Themes: reference/modules/Themes/README.md | ||
| - Admin Theme Conventions: reference/themes/admin-theme.md | ||
| - Creating an Admin Theme: reference/themes/create-admin-theme/README.md |
There was a problem hiding this comment.
I think you should leave the guide in the Guides section. There we have our step-by-step instructions, while the reference documentation is here.
There was a problem hiding this comment.
I think you should leave the guide in the Guides section. There we have our step-by-step instructions, while the reference documentation is here.
Hi @gvkries, thanks for your feedback!
The reason I moved the folder was to follow the suggestion in issue #15299, which aimed to reorganize theme-related documentation into a new Reference section.
However, I totally agree with your point that step-by-step instructions fit better in the Guides section. Would you prefer if I:
-
Move the "Creating an Admin Theme" guide back to the Guides section.
-
Only use the new reference/themes folder to host technical documentation (e.g., explaining IThemeService, AdminController, and the [Admin] attribute) as requested in the issue?
Let me know what you think, and I will update the PR accordingly!
There was a problem hiding this comment.
Fair point, but I think moving it below Reference -> Modules -> CMS Modules makes it too hard to discover. Maybe we can move everything theme-related into a new category, e.g. Reference -> Themes.
There was a problem hiding this comment.
Fair point, but I think moving it below
Reference -> Modules -> CMS Modulesmakes it too hard to discover. Maybe we can move everything theme-related into a new category, e.g.Reference -> Themes.
That makes perfect sense! Creating a dedicated Reference -> Themes category will definitely make theme-related information much easier to find.
I will organize the content as follows:
- Move the 'Creating an Admin Theme' guide into Reference/Themes.
- Keep the new technical documentation ([Admin] attribute, IThemeService, etc.) in Reference/Themes as well.
I'll update the PR right away. Thank you!
There was a problem hiding this comment.
But you have to keep in mind that we also have this Create a theme (for front-end themes) in the "Getting Started" section. Maybe we need a broader approach for the entire theming (not just admin themes). I think it's easier to leave the overall structure as it is for now and add the reference documentation first. We can then restructure everything together in a follow-up PR.
There was a problem hiding this comment.
But you have to keep in mind that we also have this
Create a theme(for front-end themes) in the "Getting Started" section. Maybe we need a broader approach for the entire theming (not just admin themes). I think it's easier to leave the overall structure as it is for now and add the reference documentation first. We can then restructure everything together in a follow-up PR.
Hi @gvkries, thank you for the clarification!
That makes a lot of sense. Reorganizing the entire theming structure is indeed a bigger task and it's better to keep it for a separate PR to avoid overcomplicating this one.
I will revert the folder structure and the mkdocs.yml changes. I'll focus solely on adding the new reference documentation for the [Admin] attribute and IThemeService as requested in the issue.
I'll let you know once the PR is updated. Thanks!
There was a problem hiding this comment.
But you have to keep in mind that we also have this
Create a theme(for front-end themes) in the "Getting Started" section. Maybe we need a broader approach for the entire theming (not just admin themes). I think it's easier to leave the overall structure as it is for now and add the reference documentation first. We can then restructure everything together in a follow-up PR.
Hi @gvkries, I have completed the requested changes:
- Moved the Creating an Admin Theme guide back to the Guides section.
- Restored the original structure in mkdocs.yml.
- Added a new Admin Theme Conventions page in Reference -> Themes covering the [Admin] attribute and IThemeService.
- Fixed broken documentation links and resolved all build warnings.
- Updated the branch with the latest changes from main.
The documentation now builds correctly and follows the project's standards. Ready for your review!
e408140 to
369a0f3
Compare
Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
Co-authored-by: Hisham Bin Ateya <hishamco_2007@yahoo.com>
|
I've applied the suggestions. Ready for another review. Thanks! @hishamco |
|
Hi @hishamco and @MikeAlhayek, I'm sorry to bother you, but it seems the CI checks are pending approval to run. Whenever you have a moment, could you please trigger them? Thank you for your time! |
|
No need to ping, just ask for a review from the top right menu |
gvkries
left a comment
There was a problem hiding this comment.
Please revert the formatting changes in global.json and mkdocs.yml. That just adds unnecessary noise and our files are formatted with two spaces only.
|
Thanks @JasonPG2007 |
|
Congratulations on your first PR merge! 🎉 Thank you for your contribution! We're looking forward to welcoming other contributions of yours in the future. @all-contributors please add @JasonPG2007 for code. If you like Orchard Core, please star our repo and join our community channels. |
|
@github-actions[bot] I've put up a pull request to add @JasonPG2007! 🎉 |
My pleasure! |
Close #15299
Description
This PR addresses the request to document the Admin Theme conventions and reorganize the related documentation.
Changes:
admin-theme.mdinreference/themesto document the[Admin]attribute, naming conventions (AdminController), and Razor Pages folder structure.create-admin-themeguide fromguides/toreference/themes/.reference/README.mdto include links to the new and relocated documentation.Technical Note:
Verified the logic in
AdminZoneFilter.csandAdminThemeSelector.csto ensure the documented conventions accurately reflect the underlying implementation.I've also checked the relative links in the moved files to ensure they work correctly in the new location.