Skip to content

Conversation

@RichSutherSky
Copy link
Contributor

This pull request makes a small change to the layout of the modal bottom sheet header in the Compose implementation. The main update replaces a fixed height with a more flexible sizing approach for the header component.

Before After
Normal before normal after normal
Large before large after large
  • Layout improvement:
    • Replaced the use of Modifier.height(BpkSpacing.Lg) with Modifier.wrapContentSize() for the modal bottom sheet header, allowing the header to size itself based on its content instead of a fixed height.
    • Removed the unused import of height and added wrapContentSize to the imports in BpkModalBottomSheetImpl.kt.

Remember to include the following changes:

  • Component README.md
  • Tests

If you are curious about how we review, please read through the code review guidelines

Copilot AI review requested due to automatic review settings January 7, 2026 11:39
@RichSutherSky RichSutherSky added the patch A backwards compatible change/fix label Jan 7, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes the modal bottom sheet header height to be content-based instead of fixed, improving support for text scaling and accessibility. The change allows the header to adapt to its content size rather than being constrained to a fixed height.

  • Replaced fixed height constraint with wrapContentSize() to allow header to size based on content
  • Updated imports to reflect the modifier change

@skyscanner-backpack-bot
Copy link
Contributor

Warnings
⚠️ One or more component files were updated, but the tests weren't updated. If your change is not covered by existing tests please add snapshot tests.
⚠️

One or more component files were updated, but the docs screenshots weren't updated. If the changes are visual or it is a new component please regenerate the screenshots via ./gradlew recordScreenshots.

⚠️

One or more component files were updated, but README.md wasn't updated. If your change contains API changes/additions or a new component please update the relevant component README.

Generated by 🚫 Danger Kotlin against 60246a6

@skyscanner-backpack-bot
Copy link
Contributor

Warnings
⚠️ One or more component files were updated, but the tests weren't updated. If your change is not covered by existing tests please add snapshot tests.
⚠️

One or more component files were updated, but the docs screenshots weren't updated. If the changes are visual or it is a new component please regenerate the screenshots via ./gradlew recordScreenshots.

⚠️

One or more component files were updated, but README.md wasn't updated. If your change contains API changes/additions or a new component please update the relevant component README.

Generated by 🚫 Danger Kotlin against 705ee7c

@skyscanner-backpack-bot
Copy link
Contributor

Warnings
⚠️ One or more component files were updated, but the tests weren't updated. If your change is not covered by existing tests please add snapshot tests.
⚠️

One or more component files were updated, but the docs screenshots weren't updated. If the changes are visual or it is a new component please regenerate the screenshots via ./gradlew recordScreenshots.

⚠️

One or more component files were updated, but README.md wasn't updated. If your change contains API changes/additions or a new component please update the relevant component README.

Generated by 🚫 Danger Kotlin against 640012b

@skyscanner-backpack-bot
Copy link
Contributor

Warnings
⚠️ One or more component files were updated, but the tests weren't updated. If your change is not covered by existing tests please add snapshot tests.
⚠️

One or more component files were updated, but the docs screenshots weren't updated. If the changes are visual or it is a new component please regenerate the screenshots via ./gradlew recordScreenshots.

⚠️

One or more component files were updated, but README.md wasn't updated. If your change contains API changes/additions or a new component please update the relevant component README.

Generated by 🚫 Danger Kotlin against 25d0cb1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

patch A backwards compatible change/fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants