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

Don't look for a parent if you're the parent #25

Closed
wants to merge 1 commit into from

Conversation

Link2Twenty
Copy link

Allow navigating to contentObjects without crash

Allow navigating to contentObjects without crash
@moloko
Copy link
Contributor

moloko commented May 29, 2019

@Link2Twenty could you check this hasn't already been resolved by #24

@Link2Twenty
Copy link
Author

I don't think #24 fixes this, this patch allows the id to be a contentObject as running findAncestor("contentObjects") on a contentObject returns undefined

@moloko
Copy link
Contributor

moloko commented May 29, 2019

@Link2Twenty what version of the adapt framework are you using this with?

@Link2Twenty
Copy link
Author

@moloko 4.2.0

@moloko
Copy link
Contributor

moloko commented May 29, 2019

it should work OK then as if findAncestor does return undefined, the check on line 99 will cause it to return with no further...

TBH I need to get #24 merged anyway as I know for sure it fixes #23 - and when I do that I'm afraid your PR will be out of date as it's based on code that's going to be very different after the merge.

So it might be best if I release the new version, you test again against that and let me know if there's still a problem?

Apologies for having left that PR unmerged for so long...

@Link2Twenty Link2Twenty closed this Oct 9, 2020
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

Successfully merging this pull request may close these issues.

2 participants