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

Govuk js style #5855

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,13 @@
= govuk_table_thead do
= govuk_table_row do
= govuk_table_th do
= govuk_link_to t('.select_all'), '#', class: 'select-all', data: { 'all-checked': 'false' }, 'aria-label': t('.select_all_label')
.govuk-form-group{ data: { module: 'govuk-select-all', 'select-all-class': 'select-all', 'collection-class': 'select-all-box' } }
.govuk-checkboxes.govuk-checkboxes--small{ data: { module: 'govuk-checkboxes' } }
.govuk-checkboxes__item
%input.govuk-checkboxes__input.select-all{ type: :checkbox, name: 'select-all', id: 'select-all', selected: false }
%label.govuk-label.govuk-checkboxes__label{ for: 'select-all' }
%span.govuk-visually-hidden
Select all
= govuk_table_th do
= t('.case_number')
= govuk_table_th do
Expand All @@ -65,7 +71,7 @@
.govuk-form-group.error-message-container
.govuk-checkboxes.govuk-checkboxes--small{ 'data-module': 'govuk-checkboxes' }
.govuk-checkboxes__item
= b.check_box(class: 'govuk-checkboxes__input')
= b.check_box(class: 'govuk-checkboxes__input select-all-box')
= b.label(class: 'govuk-label govuk-checkboxes__label'){ t('.choose_label_html', case_number: claim.case_number) }

- claim.injection_error do |message|
Expand Down
2 changes: 1 addition & 1 deletion app/views/layouts/application.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
%script
!= ga_outlet

%body{ class: "govuk-template__body controller-#{controller.controller_name}" }
%body{ class: "govuk-template__body controller-#{controller.controller_name} govuk-frontend-supported" }
:javascript
document.body.className = ((document.body.className) ? document.body.className + ' js-enabled' : 'js-enabled');

Expand Down
2 changes: 1 addition & 1 deletion app/views/shared/_determinations_form.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
%div
= validation_error_message(@error_presenter, :determinations)

= govuk_table( id: 'determinations', class: 'js-cw-claim-assessment', data: { apply_vat: "#{claim.apply_vat}", vat_url: vat_path(format: :json), submitted_date: claim.vat_date(:db), scheme: claim.agfs? ? 'agfs' : 'lgfs' }) do
= govuk_table( id: 'determinations', class: 'js-cw-claim-assessment', data: { module: 'govuk-determination', apply_vat: "#{claim.apply_vat}", vat_url: vat_path(format: :json), submitted_date: claim.vat_date(:db), scheme: claim.agfs? ? 'agfs' : 'lgfs' }) do
= govuk_table_caption(class: 'govuk-visually-hidden') do
%h3.govuk-heading-m
= t('.assessment_summary')
Expand Down
21 changes: 0 additions & 21 deletions app/webpack/javascripts/modules/Modules.SelectAll.js

This file was deleted.

This file was deleted.

86 changes: 86 additions & 0 deletions app/webpack/javascripts/modules/determination.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
/**
* Determination
*
* Calculate totals for the assessment summary on case_workers/claims/<claim-id>
*
*
* <div data-module="govuk-determination" data-apply-vat="true" data-vat-url="/vat.json" data-submitted-date="2023-06-27" data-scheme="agfs">
* <!-- Calculated values -->
* <span class="js-total-exc-vat-determination">...</span>
* <span class="js-vat-determination"...</span>
* <span class="js-total-determination">...</span>
*
* <!-- Fields used for the calculation -->
* <input id="claim_assessment_attributes_fees" />
* <input id="claim_assessment_attributes_expenses" />
* <input id="claim_assessment_attributes_disbursements" />
*
* <!-- Field for LGFS claims -->
* <input class="js-lgfs-vat-determination"id=" claim_assessment_attributes_vat_amount">£0.00</span>
* </div>
*/

export class Determination {
/**
* @param {Element} $module - HTML element to use for component
*/
constructor ($module) {
if (($module instanceof window.HTMLElement) && document.body.classList.contains('govuk-frontend-supported')) {
this.$module = $module
}
}

/**
* Initialise component
*/
init () {
if (!this.$module) { return }

const $module = this.$module

this.$totalExclVat = $module.querySelector('.js-total-exc-vat-determination')
this.$totalVat = $module.querySelector('.js-vat-determination')
this.$LgfsVat = $module.querySelector('.js-lgfs-vat-determination')
this.$totalInclVat = $module.querySelector('.js-total-determination')
this.scheme = $module.dataset.scheme
// Should this be this.applyVat?
this.ajaxVat = $module.dataset.applyVat
this.vatUrl = $module.dataset.vatUrl
this.vatDate = $module.dataset.submittedDate

const fieldIds = [
'fees',
'expenses',
'disbursements',
'vat_amount'
]

this.fields = fieldIds.map(id => document.querySelector(`#claim_assessment_attributes_${id}`)).filter(field => field)
this.fields.forEach(element => element.addEventListener('change', () => {
this.calculateTotalRows()
return true
}))
}
Copy link
Contributor

@imtiazAhmd imtiazAhmd Oct 13, 2023

Choose a reason for hiding this comment

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

You can try

const fieldIds = [
  'fees',
  'expenses',
  'disbursements',
  'vat_amount'
];

this.fields = fieldIds.map(id => document.querySelector(`#claim_assessment_attributes_${id}`))
                     .filter(field => field);

this.fields.forEach(field => field.addEventListener('change', () => this.calculateTotalRows()));


calculateTotalRows () {
const total = this.fields.reduce((n, field) => n + (parseFloat(field?.value) || 0.0), 0).toFixed(2)
return this.applyVat(total).then(data => {
this.$totalExclVat.innerHTML = data.net_amount
if (this.$totalVat) { this.$totalVat.innerHTML = data.vat_amount }
this.$totalInclVat.innerHTML = data.total_inc_vat
})
}

Copy link
Contributor

Choose a reason for hiding this comment

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

You can use async await here. Makes applyVat to return a promise that can be awaited. Also if you are using parsedFloat only here that also can be removed/ or not !

async function calculateTotalRows() {
  const total = this.fields.reduce((n, field) => n + (parseFloat(field?.value) || 0.0), 0).toFixed(2);
  const { net_amount, vat_amount, total_inc_vat } = await this.applyVat(total);
  this.$totalExclVat.innerHTML = net_amount;
  if (this.$totalVat) { this.$totalVat.innerHTML = vat_amount; }
  this.$totalInclVat.innerHTML = total_inc_vat;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried making this into an async function and now SonarCloud is flagging a bug at the point at which it is being called: "Promise returned in function argument where a void return was expected." I remember when I tried this first I was getting similar issues and couldn't work out how to resolve it so I opted for it not being async. Is there a way to fix this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Try adding return Promise.resolve();

async applyVat (netAmount) {
const params = new URLSearchParams({
scheme: this.scheme,
lgfs_vat_amount: this.$LgfsVat?.value,
date: this.vatDate,
apply_vat: this.ajaxVat,
net_amount: netAmount
})
const response = await fetch(this.vatUrl + '?' + params.toString())

Copy link
Contributor

Choose a reason for hiding this comment

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

Not a suggestion but for reference, we can also remove string concatenation by using backtick.

const response = await fetch(`${this.vatUrl}?${queryString}`);

return await response.json()
}
}
Loading