Skip to content

Draft: Chore/move logic out of nunjucks#162

Open
EdenJia-Capgemini wants to merge 13 commits into
mainfrom
chore/move-logic-out-of-nunjucks
Open

Draft: Chore/move logic out of nunjucks#162
EdenJia-Capgemini wants to merge 13 commits into
mainfrom
chore/move-logic-out-of-nunjucks

Conversation

@EdenJia-Capgemini
Copy link
Copy Markdown
Collaborator

@EdenJia-Capgemini EdenJia-Capgemini commented Feb 19, 2026

Description

Currently moved logic out or results.njk and history.njk into its separate presenter.ts files. Also refactored results.njk to be more in parts for easier access for future development. Looking for feedback on changes so far since the size of change seems more than expected.

edit: It has been decided that the logic transfer so far is a good size for this PR.

Related issues: Move logic out of the Nunjucks Views

Checklist before marking as ready for review

Before marking as ready for review, please check that you have:

  • Added appropriate and passing tests for the changes I made.
  • Not broken any existing functionality or tests.
  • Not introduced any new linting or formatting errors.
  • Only added new dependencies that are necessary, do not introduce security vulnerabilities, and with tilde (~) version ranges.
  • Checked and resolved UI accessibility issues using Axe devtools. Unresolved issues are described above.
  • Updated the documentation if necessary.

Check the contributing guidelines for more details.

Screenshots (if appropriate)

@EdenJia-Capgemini EdenJia-Capgemini marked this pull request as draft February 19, 2026 12:19
@EdenJia-Capgemini EdenJia-Capgemini self-assigned this Feb 19, 2026
Comment thread src/routes/presenters/results-page-structure.presenter.ts Fixed
@EdenJia-Capgemini EdenJia-Capgemini marked this pull request as ready for review March 13, 2026 11:26
{ text: "Remove" }
],
rows: sharingVM.ownerRows
}) }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get an error in the console when trying to share the prototype with another user, and it won't let me add them. Does sharingVM.ownerRows include the hidden user and "Not shared with any users" row?

Uncaught (in promise) TypeError: Cannot read properties of null (reading 'cloneNode')
    at HTMLInputElement.<anonymous> (prototype-sharing.js:176:17)

],
rows: sharingVM.ownerRows
}) }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should be <p class="sharing-last-updated govuk-body"></p> here, to show the user that the sharing settings are updated when they click save.

Comment thread .vscode/settings.json
"git.branchProtection": ["main"]
"git.branchProtection": ["main"],
"[nunjucks]": {
"editor.defaultFormatter": "okitavera.vscode-nunjucks-formatter"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this to extensions.json please?

<ol>
{% for item in structureVM.list %}
<li>
<strong>{{ item.answerType }}</strong>: {{ item.questionText }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was previously wrapped in <a> tags, that when clicked jumped the preview to the question - can you re-add this?

Comment thread views/history.njk
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This page looks to be missing its title and some of the filters; the filters should also be full-width.

Comment thread views/history.njk
</div>
</div>

{{ govukTable({ head: header, rows: prototypeRows }) }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add firstCellIsHeader: true back into this?

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.

3 participants