Skip to content

Conversation

@ianjon3s
Copy link
Contributor

@ianjon3s ianjon3s commented Dec 19, 2025

What does this PR do?

  • Updates expandable checklists to output images
  • Also adds image output to "exclusive" option

Testing:
https://5941.planx.pizza/a-new-team/checklists/preview

@github-actions
Copy link

Pizza

Deployed 573fc14 to https://5941.planx.pizza.

Useful links:

@ianjon3s ianjon3s marked this pull request as ready for review December 19, 2025 12:22
@ianjon3s ianjon3s requested a review from a team December 19, 2025 12:22
Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

This does work as expected, but is a little bit of an anti-pattern I think. We're prop-drilling hasImages into multiple components when we could reach for getLayout() as we always have the associated option or groupedOptions. There's a potential here for bugs as we have two (possibly diverging) sources of truth - how should a component render where hasImages={true} but there are no img properties, or vice-versa?

Our current layouts consist of -

export enum ChecklistLayout {
Basic,
Grouped,
Images,
}

This PR would mean that this list is no longer a discrete description of the possible layouts - we can have grouped layouts with and without images.

I'd recommend taking a look at a solution which introduces a new layout, and updates the logic of getLayout() to reflect this.

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