-
Notifications
You must be signed in to change notification settings - Fork 1
[CCM-10893] Header with account info #629
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
Conversation
…emplate-management into feature/CCM-10893-header-with-account-info
…emplate-management into feature/CCM-10893-header-with-account-info
…emplate-management into feature/CCM-10893-header-with-account-info
| @@ -0,0 +1,3 @@ | |||
| export const truncate = (text: string, maxLength = 50): string => { | |||
| return text.length > maxLength ? text.slice(0, maxLength - 1) + '…' : text; | |||
| }; | |||
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.
Might be good to trim any whitespace from the end after slicing so you don't end up with word …
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.
I'm not sure on this one, but maybe always trim trailing whitespace before doing anything else. If you only discarded whitespace, you don't need an ellipsis
|
@nicki-nhs I think it would be good to add a component/sandbox test. You would need to update the pre-token-generation lambda in this repo to match your updates in IAM. It's a simplified version which takes all its input from user attributes instead of SSM + user attributes. The client name would need to be added to users as a user attribute. You can do this by adding |
Adding name etc. to the accessibility test user would also probably be worthwhile |
…emplate-management into feature/CCM-10893-header-with-account-info
| const $ClientFeatures = schemaFor<ClientFeatures>()( | ||
| z.object({ | ||
| proofing: z.boolean(), | ||
| // TODO: CCM-11148 Make routing required |
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.
I'm not sure why either flag should be required here
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.
I can change proofing too if you want, as long as we've got fallback logic in place..
if we dont set proofing, I'm guessing it defaults to false somewhere?
and if we change it here, does that then mean we need to add testing round the proofing feature flag for this ticket?
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.
I don't think it's worth changing proofing unless we find we need to or some convenient opportunity comes up
| 'preferred_username', | ||
| 'given_name', | ||
| 'family_name', | ||
| ] as UserIdentityAttributes[], |
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.
I think the assertion is unnecessary
Description
nhs-frontendto pre-release 10^10.0.0-internal.0to bring in the new header with account from the design systemnhs-frontendAuthLinkto remove redundant wrappingdivtruncate- truncates strings and replaces excess characters with ellipsestoken-utils- for client side handling of jwt tokens, and addgetIdTokenClaimsfunctionamplify-utilstoidTokeningetSessionServertoken-utilswhere relevantHeaderWithAccountcomponent whichnhs-frontendroutingfeature is true infeaturesprop)HeaderinClientLayoutwith the newHeaderWithAccountroutingfeature flag intoClientFeaturestype (doesnt yet do anything)idin the tests withdata-testidand adding conditional for empty itemsContentRendererto account for the aboveFooterto remove redundant html containers and update to use latest classes etc fromnhs-frontendContext
Users need to see the name of the client displayed in the header so that they can see which client they are currently creating or updating resources for
Type of changes
Checklist
Sensitive Information Declaration
To ensure the utmost confidentiality and protect your and others privacy, we kindly ask you to NOT including PII (Personal Identifiable Information) / PID (Personal Identifiable Data) or any other sensitive data in this PR (Pull Request) and the codebase changes. We will remove any PR that do contain any sensitive information. We really appreciate your cooperation in this matter.