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

PP-12444 refactor settings layout #4384

Merged
merged 2 commits into from
Dec 13, 2024
Merged

Conversation

nlsteers
Copy link
Collaborator

@nlsteers nlsteers commented Dec 12, 2024

WHAT

  • add service-settings-pane css class to enforce consistent width across all settings views
  • remove hint-and-body-width css class
  • update base settings layout to include the back link and error summary macros
  • update existing views to conform with new layout behaviour
  • refactor email notifications custom paragraph controller to match common code style
  • fix routing bug where a service could navigate to stripe details summary in test mode once live
  • fix layout bug where long words would break out of the system messages component
  • fix layout bug where long task names would cause the task status to jump onto multiple lines
  • fix controller bug where entering a welsh service name that fails validation would cause the 'remove welsh service name' button to unsuccessfully attempt to remove the english service name

SCREENS

  • pull the branch, too many changes to screenshot

@nlsteers nlsteers force-pushed the pp-12444/ticket_review_comments branch from c6692e1 to 716ffa8 Compare December 12, 2024 17:27
- add `service-settings-pane` css class to enforce consistent width across all settings views
- remove `hint-and-body-width` css class
- update base settings layout to include the back link and error summary macros
- update existing views to conform with new layout behaviour
- refactor email notifications custom paragraph controller to match common code style
- fix layout bug where long words would break out of the system messages component
- fix layout bug where long task names would cause the task status to jump onto multiple lines
- fix controller bug where entering a welsh service name that fails validation would cause the 'remove welsh service name' button to unsuccessfully attempt to remove the english service name
@nlsteers nlsteers force-pushed the pp-12444/ticket_review_comments branch from 716ffa8 to 40f2af5 Compare December 12, 2024 17:47
Comment on lines +40 to +41
text: "Send payment confirmation emails"
},
Copy link
Contributor

Choose a reason for hiding this comment

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

As above

Comment on lines 40 to 42
hint: {
text: 'Users can choose to enter an email address – if they do, they can receive notifications'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

According to proper json formatting the indent should be

a: {
  b: 'whatever'
}

so lines 41-42 should be indented to the right.

Comment on lines 52 to 53
text: 'Users do not have the option to enter an email address – they will not receive notifications'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

As above

Comment on lines 27 to 28
text: 'Users must enter an email address to complete a payment – they can receive notifications'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

As above

Comment on lines 34 to 36
hint: {
text: 'Users can choose to enter an email address – if they do, they can receive notifications'
}
Copy link
Contributor

Choose a reason for hiding this comment

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

As above

Comment on lines +45 to +46
text: "Permission level",
classes: "govuk-fieldset__legend--s"
Copy link
Contributor

Choose a reason for hiding this comment

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

single quotes


{{ govukButton({
text: "Send invitation email"
Copy link
Contributor

Choose a reason for hiding this comment

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

single quotes

Comment on lines +14 to +15
idPrefix: "confirmRemoveUser",
name: "confirmRemoveUser",
Copy link
Contributor

Choose a reason for hiding this comment

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

single quotes

Comment on lines +19 to +22
text: "Are you sure you want to remove " + email + "?",
isPageHeading: true,
classes: "govuk-fieldset__legend--l"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

single quotes

Comment on lines +25 to +35
text: "They will no longer have access to this service."
},
items: [
{
value: "yes",
text: "Yes",
attributes: {'data-cy': 'yes-radio'}
},
{
value: "no",
text: "No",
Copy link
Contributor

Choose a reason for hiding this comment

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

single quotes

@nlsteers nlsteers merged commit 2b752a0 into master Dec 13, 2024
13 of 14 checks passed
@nlsteers nlsteers deleted the pp-12444/ticket_review_comments branch December 13, 2024 14:14
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.

2 participants