cloudscribe/cloudscribe.Web.Navigation

Add support for Razor Pages in Navigation Node

Closed this issue · 20 comments

pbros commented

Currently, links are generated when a Controller and Action is specified. However, when designing a site using Razor Pages, we specify a Page directly.

In the NavigationViewModel.AdjustUrl(), it should be a matter of using urlHelper.Page() instead of urlHelper.Action(), and adding a new property on NavigationNode.

are you adding your pages via the navigation.xml file?
for razor pages can't you just use url instead of controller and action or named route?

pbros commented

I am using the navigation.xml.

You're right that I can use the url, but having a property for "Page" would make it more similar to the asp- tag helpers (https://docs.microsoft.com/en-us/aspnet/core/mvc/views/tag-helpers/built-in/anchor-tag-helper?view=aspnetcore-2.2#asp-page) and make it compatible with the use of the "area" tag (https://docs.microsoft.com/en-us/aspnet/core/mvc/views/tag-helpers/built-in/anchor-tag-helper?view=aspnetcore-2.2#usage-in-razor-pages)

we do have support for area already for controllers, but see what you mean, pages can also be in areas.
I'm happy to add support for pages just not sure how soon I can get to it myself. I really have not done much with razor pages myself.

If you are interested in implementing it and making a pull request, I'm open to that but would need a signed agreement as indicated here https://github.com/cloudscribe/cloudscribe/blob/master/CONTRIBUTING.md
in order to review or merge a pull request.

Ideally there should be another demo app in the solution for razor pages.

Don't know if I should create a new issue (not really an issue, more a feature request), but it would be super awesome if the side navigation could be generated dynamically from a Razor Pages folder such as a docs folder for example.

Such a feature can be found for example in some documentation generators which use Front Matter headers in md files and js files to configure the navbar dynamically.

@os1r1s110 that sounds like a great idea, but should be built by someone who is using razor pages in their projects. I haven't done much with Razor pages, mostly using mvc or api style in my projects. I will eventually get to the issue posted above if no-one else does but I can't promise to do it soon.

Your request is separate but sort of depends on the enhancement requested on this issue.
I can offer advice how to do it. You can implement INavigationTreeBuilder, I've implemented one in my SimpleContent cms that creates a tree from cms page stored in Db. The same approach can be used for what you want. Then you can declare a node in the navigationxml that uses your treebuilder, like this:

 <NavNode key="Home"
      controller="Home"
      action="Index"
      text="Home"
       componentVisibility="none"
      treeBuilderName="cloudscribe.SimpleContent.Services.PagesNavigationTreeBuilder">
<Children>...more nodes can be declared here...</Children></NavNode>

However as mentioned above for any significant code contributions I need a signed agreement that allows us to keep purity of copyright ownership and therefore ability to transfer copyright in case we can get this project accepted into .NET Foundation in the future.

If you would like to send a signed agreement and contribute that work it would be great. I will accept your other pull request without that since it is very small.

Alternatively you can also implement INavigationTreeBuilder in your own project if you don't want to sign an agreement.

I am actually in the middle of trying to implement just that (the INavigationTreeBuilder) to provide this very feature. I have no problem signing the agreement (where can it be found if it already exists?)

Also, I will do my best to provide a first working prototype, but I might need help cleaning/making it more "production ready" as I don't have tons of experience and I'm kind of "toying around" with it atm.

@os1r1s110 great, there is a link to the agreement in the contributing guide here https://github.com/cloudscribe/cloudscribe/blob/master/CONTRIBUTING.md

a direct link: https://github.com/cloudscribe/cloudscribe/blob/master/copyright-assignment.txt

If you can sign and scan as pdf and send to info@cloudscribe.com that would be great and ideally mail an original to Source Tree Solutions, LLC PO Box 1978, Concord, NC 28026

I'll be glad to review and provide feedback to make it production ready.
Will you also take on the request in this issue which your request kind of depends on?
Ideally it would be good to have another demo web in the solution that is a Razor pages project to make it easy for others to see how to use it in Razor pages projects, so if you could do that as well it would be fantastic.

I'm not sure my request really depends on @pbros feature (hence why I wondered if I would have been better creating a new issue).

The reason being that for my feature request to work, there is no need for the navigation.xml except for the main parent node, from which the pages will be discovered using the actionDescriptorCollectionProvider.

For now I mainly focus on supporting my main use case (which is building the navigation tree from razor pages specific folder discovery), and I will do as much as I can to let flexibility in its usage). So I don't say I won't do the other feature request, but I can't promise it will be done in a timely manner as I will try to do it in my spare time.

@os1r1s110 fair enough, I see your point. I just thought your treebuilder would want to be creating nodes that have page as a property like controller and acction are node properties, but if you are just resolving the url then you don't need it. Actually in my treebuilder for simplecontent I'm also resolving the url not setting controller or action so I see you don't really need that.

I understand how you see it (getting the nodes in the navigation.xml which have a Page property and use that to generate the url, that could be good too), but in my scenario, the whole point is to avoid having to manually configure each item within the navigation.xml or json file.

Should I create a new separate issue for my use case?
As after reading your comment, I think I understand better what @pbros was looking for and it's not exactly similar, even though also interesting as a feature :)

@os1r1s110 yes a separate issue for what you are working on would be good. I agree it doesn't depend on this issue.

For now I will assign this issue to myself to make it clear that this will eventually get done, but not sure how soon I will get to it so it is up for grabs if someone else wants to make it a priority, and I would be happy to re-assign it.

pbros commented

Hi @joeaudette, I might be able to make the modification myself. I'll let you know. That being said, what @os1r1s110 is proposing might actually be more useful for me!

pbros commented

*closed by mistake

I thought of some potential challenges for what @os1r1s110 is wanting to do, was waiting for the new issue to be created to continue the discussion.

Quick update, I'm trying to find a way to implement the feature I talked about but there are some concerns that I'm stumbling on and I'd like your opinion on it.

  1. If I go through the directory to find pages (or use the actionDescriptorCollectionProvider), there's no way to enforce a certain order in the navigation menu (which might be problematic). Is that a valid concern? One potential solution would be to use specially formatted headers in the razor pages in which we could specify the order, but I don't know if that's overkill or not.

  2. Even if I do implement a custom header in the razor pages that is parsed when generating the navigation menu, there is still no way to define in which order each folder will be rendered in navigation.... would anyone have a suggestion?

^I thought maybe we could manually add a node in the navigation.xml for each folder, and then use the pages discovery to generate the sublinks, but that would defeat part of the "automatic discovery" feature....

Sorry I commented here, is there a way to transfer the last comments to the new issue? Or should I simply copy them over?

I've implemented this and published an updated nuget and also added a razor pages demo in the solution
https://www.nuget.org/packages/cloudscribe.Web.Navigation/2.1.8

It might be a little while before the new nuget is indexed on nuget.org and available.

I'm closing this for now feel free to open new issue if you have any trouble with the latest nuget.

pbros commented

Seems like there is a bug finding the current node. The FindByUrl method does not take into consideration Pages and Areas.

I added the following logic locally (similar to and right after the Action & Controller check) and it now works!

if ((!string.IsNullOrEmpty(n.Value.Page)))
                {
                    targetUrl = urlHelper.Page(n.Value.Page, new { area = n.Value.Area });

                    if (targetUrl == null) return false; // check for null in case action cannot be resolved
                    if (urlToMatch.EndsWith("/"))
                    {
                        targetUrl = targetUrl + "/";
                    }
                    if (targetUrl.IndexOf(urlToMatch, StringComparison.OrdinalIgnoreCase) >= 0)
                    { return true; }

                }
pbros commented

Seems like you added the same bit of code in 2.1.9!