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

(Mailables): Support Simultaneous Small and Large Print #925

Merged
merged 2 commits into from
Jun 22, 2023

Conversation

miles-grant-ibigroup
Copy link
Collaborator

Description:

The mailables large print feature currently disables the small print on the resulting pdf. This PR corrects this, allowing for simultaneous large and small prints. Unfortunately, there is no way to change the count for the second size. Not sure how best to solve this.

PR Checklist:

  • [no] Does the code follow accessibility standards (WCAG 2.1 AA Compliant)?
  • [no] Are all languages supported (Internationalization/Localization)?
  • [no] Are appropriate Typescript types implemented?

Signed-off-by: miles-grant-ibigroup <miles.grant@ibigroup.com>
Copy link
Contributor

@philip-cline philip-cline left a comment

Choose a reason for hiding this comment

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

lgtm

Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup left a comment

Choose a reason for hiding this comment

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

Could you get rid of the checkboxes and just have two inputs? One for small format one for large?
Having trouble understanding why you'd want to select both checkboxes without being able to control the number for each, but it it makes sense in context I trust you

}
return rows
})
.flat()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpicky, but could just change the above map to flatMap

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is much cleaner thanks!

Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup left a comment

Choose a reason for hiding this comment

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

Whoops, meant to approve! Nothing blocking

@miles-grant-ibigroup miles-grant-ibigroup merged commit cb67e30 into dev Jun 22, 2023
@miles-grant-ibigroup miles-grant-ibigroup deleted the mailalbes-both-small-and-large-print branch June 27, 2023 14:17
@miles-grant-ibigroup miles-grant-ibigroup restored the mailalbes-both-small-and-large-print branch October 19, 2023 16:14
@miles-grant-ibigroup miles-grant-ibigroup deleted the mailalbes-both-small-and-large-print branch October 19, 2023 16:21
miles-grant-ibigroup added a commit that referenced this pull request Oct 19, 2023
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.

3 participants