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

Modify code to always have an ld json element, may be empty #1188

Merged
merged 1 commit into from
Aug 23, 2021

Conversation

kinow
Copy link
Collaborator

@kinow kinow commented Aug 21, 2021

I think this issue happens due to a mix of Twig and JS code. When you visit a vocabulary, the value of request.page == 'page' is false, since request.page has the value "vocab".

But if you click on a concept, it will trigger the code to load concept mappings without changing the value of request.page. That code won't find the JSON-LD script element, and result in a JS error. If you reload the page, now the request.page will be page and now the concept mappings will be displayed correctly.

This pull request simply modifies the template to always have a JSON-LD script element, that may be empty ({}). This way the code should work no matter what page is trying to load concept mappings. Tested locally with YSO vocabulary.

Closes #1016

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
2.3% 2.3% Duplication

@kouralex kouralex added this to the Next Tasks milestone Aug 23, 2021
@kouralex kouralex self-assigned this Aug 23, 2021
@kouralex
Copy link
Contributor

Perfect! Thank you @kinow for implementing this.

PS. I assume this also fixed #1186 ? 😃

@kouralex kouralex merged commit 3d05609 into NatLibFi:master Aug 23, 2021
@kouralex kouralex modified the milestones: Next Tasks, 2.12 Aug 23, 2021
@kinow kinow deleted the always-have-a-jsonld-element branch August 23, 2021 10:12
@kinow
Copy link
Collaborator Author

kinow commented Aug 23, 2021

Perfect! Thank you @kinow for implementing this.

PS. I assume this also fixed #1186 ? 😃

Yup, should have fixed both. Now after the release we can ask Henk to confirm it's working OK 🎉 thanks @kouralex !

@osma osma added the bug label Sep 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unhandled JavaScript error causes mapping properties to be unable to load
3 participants