hmrc / hmrc-frontend

Apache License 2.0
9 stars 18 forks source link

closes: Closes #316 #317

Closed bsamray closed 10 months ago

bsamray commented 1 year ago

With this issue, we aim to add a parameter to the timeout-dialog component, just like many others such as signOutButtonText. This parameter, when set, shall result in not showing the 'Sign out' link on the modal timeout dialog. We have used this component in our UK Government citizen-facing ui app, but we need this feature in line with our design requirements

hmrc-web-operations commented 1 year ago

Thank you for submitting a pull request on this HMRC repository. A member of the HMRC team will review your changes and action accordingly.

HMRC reviewer: please reference this runbook for guidance on how to proceed.

bsamray commented 1 year ago

Hello @hmrc-web-operations , I am unable to view the runbook page - https://github.com/hmrc/build-and-deploy/blob/main/runbooks/reviewing_external_pull_requests.md above. If possible, please can you advise on how to proceed on getting this PR reviewed and merged.

bsamray commented 12 months ago

Hi @oscarduignan thanks for your email enquiring about this change. I am not sure if you received my response (I replied to that email hoping you would get it). We are needing this change for a Department for Work and Pensions citizen-facing service that is due to go live soon. And this change is important to meet the design needs in line with accessibility requirements. I fully acknowledge this is an external change to this useful open source project, but if this can be reviewed and later included (of course if deemed fit), we can just use it out of the box rather than having to do use a forked version. Thanks in advance.

oscarduignan commented 11 months ago

Hi @bsamray I've checked with the other teams that own this repo and I think we're all ok to integrate this. I'm going to create a ticket in our internal backlog to tweak the changes and get this integrated. The feature won't change much, off top of my head it's just some housekeeping bits like your package-lock.json file was generated with a version of node/npm different from what we're using at the moment which has caused it to have a much larger amount of changes than needed and the attribute needs to be a data attribute (so prefixed with data-*) we can sort that though

Can't guarantee a time we'll be able to start it but because it's a small change since you've done the bulk of the work for us I don't think it should take long (week or two)

bsamray commented 11 months ago

Thanks @oscarduignan much appreciated. Please let us know if you need us to do anything on this front. Thanks again.

bsamray commented 10 months ago

This feature is now included in the 5.52.0 through merged PR #324