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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
MMasey opened this issue Jun 21, 2016 · 10 comments
Open

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

MMasey opened this issue Jun 21, 2016 · 10 comments
Assignees
Milestone

Comments

@MMasey
Copy link

MMasey commented Jun 21, 2016

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?

@MMasey MMasey changed the title Include node Id in related Link is it is internal. Include node Id in RelatedLink if it is internal. Jun 21, 2016
@Jeavon
Copy link
Owner

Jeavon commented Jun 21, 2016

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
Copy link
Author

MMasey commented Jun 21, 2016

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
Copy link
Owner

Jeavon commented Jun 21, 2016

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
Copy link
Author

MMasey commented Jun 21, 2016

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
Copy link
Owner

Jeavon commented Jun 21, 2016

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

@MMasey
Copy link
Author

MMasey commented Jun 21, 2016

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

@Jeavon
Copy link
Owner

Jeavon commented Jun 21, 2016

@Jeavon Jeavon added this to the v3.0.0 milestone Jun 21, 2016
@Jeavon Jeavon self-assigned this Jun 21, 2016
@MMasey
Copy link
Author

MMasey commented Jun 21, 2016

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

Thanks again.

@MMasey
Copy link
Author

MMasey commented Aug 23, 2016

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

@Jeavon
Copy link
Owner

Jeavon commented Aug 23, 2016

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants