-
Notifications
You must be signed in to change notification settings - Fork 1
[CCM-11494] Alternative letter formats #772
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-11494-choose-letter-formats
frontend/src/__tests__/app/choose-templates/__snapshots__/page.test.tsx.snap
Show resolved
Hide resolved
...src/components/molecules/MessagePlanConditionalTemplates/MessagePlanConditionalTemplates.tsx
Show resolved
Hide resolved
...end/src/components/molecules/MessagePlanFallbackConditions/MessagePlanFallbackConditions.tsx
Show resolved
Hide resolved
harrim91
left a comment
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.
Just a couple of tiny things
| return null; | ||
| } | ||
|
|
||
| const accessibleFormats = ACCESSIBLE_FORMATS; |
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.
Really unimportant but why is this assignment needed?
| /** | ||
| * Collects all remaining language types from the cascade | ||
| */ | ||
| export function getRemainingLanguages(cascade: CascadeItem[]): Language[] { |
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.
nit: can these be called getCascadeLanguages rather than getRemaining... - I wasn't sure what "remaining" was referring to until I saw how the context in which the function was being called.
| ); | ||
| } | ||
|
|
||
| export function MessagePlanLanguageTemplate({ |
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.
(Opinionated): Putting these into separate files would improve readability and would discourage growing the file excessively in the future.
| /** | ||
| * Collects all remaining accessible format types from the cascade | ||
| */ | ||
| export function getRemainingAccessibleFormats( |
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.
(Opinionated): You can avoid the continue and nested ifs with something like:
export function getRemainingAccessibleFormatsAlt(cascade: CascadeItem[]): LetterType[] {
const formats = cascade
.filter((item) => item.conditionalTemplates)
.flatMap((item) => item.conditionalTemplates)
.filter((template) => 'accessibleFormat' in template && template.templateId)
.map((template) => template.accessibleFormat)
.reduce((acc, format) => acc.add(format), new Set<LetterType>());
return [...formats];
}
|
superseded by #778 |
Description
MessagePlanChannelTemplatecomponent for accessible formats and foreign language templatesScreenshots
Without templates selected:


With templates selected:
Context
As an integrator I Want to link accessible format (Large print and Other languages) to a message plan
And review it So that I can verify if the message plan is correct
Choosing accessible format letter will be possible but it is optional. The user will be able to select the template they need and also remove it or change it.
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.