-
Notifications
You must be signed in to change notification settings - Fork 2
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
Glossary terms snippet #110
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of good work. Left a number of comments and suggestions
cms/jinja2/templates/components/streamfield/glossary_section_block.html
Outdated
Show resolved
Hide resolved
… follow after the final decision is made
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, looks like there are some functional test issues but approving since those will have to be fixed to pass checks 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Nice work Kacper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. No comments apart from the existing convos already going on + the need to get one of the UI tests passing. Tested everything locally and everything works as expected.
@kacperpONS A loom video would be a nice addition, a gentle reminder.
…he latest revision instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-reviewed. Tested everything locally. All is well!
What is the context of this PR?
We want to add the Glossary term snippet and allow it to be used on Methodology and Statistical Article pages.
How to review
Make sure code looks ok, run tests.
Links
https://jira.ons.gov.uk/browse/CMS-81
Loom recording: https://www.loom.com/share/c053a155c6384b50b94f0b13a759cbd9?sid=febc7d52-7e9d-460c-a8e5-04320ad82465