Jeavon / Umbraco-Core-Property-Value-Converters

19 stars 13 forks source link

Include node Id in RelatedLink if it is internal. #13

Open MMasey opened 8 years ago

MMasey commented 8 years ago

Would you be able to add the node Id when a link is internal to the RelatedLink model, so that if using the related links for a main site navigation, you can use it to set a "current" state to the link?

Jeavon commented 8 years ago

This is happening already internally so you know that the link will always be current. See https://github.com/Jeavon/Umbraco-Core-Property-Value-Converters/blob/v2/Our.Umbraco.PropertyConverters/Models/RelatedLink.cs#L154

MMasey commented 8 years ago

Ah ok, could you explain how i would use in the instance where i would like to set a "link-current" class for example if the link matched the current page?

Usually i would create some logic that uses the link node Id and checks to see whether it matches the current page Id, then render the class="class-name" using the bool value.

Here's a very rough example

<ul>
    @foreach (var item in typedRelatedLinksConverted)
    {
        var linkTarget = (item.NewWindow) ? "_blank" : null;
        var isCurrent = item.NodeId == Model.Content.Id;
        <li><a href="@item.Link" target="@linkTarget" @(isCurrent ? "class=\" link-current\"" : "")=)>@item.Caption</a></li>
    }
</ul>
Jeavon commented 8 years ago

Interesting use case, currently you would have to compare the Url, if we add a Id property it would only have a value if the link is internal which a bit odd but that's the only solution I can think of currently...

MMasey commented 8 years ago

Yeah it is a bit of a unique one =P

I really like the UI for the related links property and found it works really nicely alongside nested content for creating multilevel site navigation. I ended up using your value converter as a basis to create my own RelatedLink model which includes the node Id if it has one, and from that i create a PageLink model, which has an IsActive (checks if the page is a parent of the current page) and IsCurrent (check is the link is the current page) boolean value. I could probably move those directly to the RelatedLink model thinking about it.

The view then never sees the Node Id value, it just asks is either of those values are true and sets a class if they are. Its the link is external is will simple be false.

I'm happy to put together a pull request with an example of my implementation if you want, although like you said though it may be a bit to much of a use case for what your package does =]

Jeavon commented 8 years ago

I'll add Id to the model, should it be nullable or should it return 0 if not an internal type?

MMasey commented 8 years ago

Awesome, I did mine so it was nullable. It seemed less likely to fail in my head.

Jeavon commented 8 years ago

Added, you can get a pre-release if you want to try it out https://ci.appveyor.com/project/JeavonLeopold/umbraco-core-property-value-converters/build/3.0.0.64/artifacts or https://www.myget.org/gallery/umbraco-core-property-value-converters (NuGet)

MMasey commented 8 years ago

Nice one, I'll try swapping mine out for it later today or tomorrow.

Thanks again.

MMasey commented 8 years ago

I've finally gotten round to updating my project with the update and it works like a charm.

Jeavon commented 8 years ago

Great!