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

Add bubbleImageMaxHeight and bubbleImageMinHeight #5236

Merged
merged 9 commits into from
Aug 6, 2024

Conversation

compulim
Copy link
Contributor

@compulim compulim commented Jul 5, 2024

Fixes #5235.

Changelog Entry

Breaking changes

  • styleOptions.bubbleImageHeight is being deprecated in favor of styleOptions.bubbleImageMaxHeight and styleOptions.bubbleImageMinHeight. The option will be removed on or after 2026-07-05
    • Image will be shown at minimium height of 180 and maximum height of 240, instead of fixed height of 240
    • To revert to previous behavior, set both values to 240

Added

  • Added styleOptions.bubbleImageMaxHeight and styleOptions.bubbleImageMinHeight for variable image height, in PR #5236, by @compulim

Description

Previously, to support IE11, we could not use object-fit. For simplicity, we make images with fixed height.

As we discontinued support of IE11, we can now making image height variable (min/max) and relies object-fit to honor the aspect ratio.

Design

The image resize logic is "enlarge image to meet fixed width, then crop/pad top/bottom to meet height."

Current design is "fixed height", it will crop the image if it is too tall, or pad the image if it is too short.

This PR will enable variable height by min/max. When min is set to 0 and max is set to Infinity, it means "fixed width and make the image as short/tall as it fit."

When the image is taller than max, it will be cropped. When the image is shorter than min, it will be padded.

When min/max are set to the same number, say, 240, it will always crop/pad the image at height of 240px and this is same as today's behavior and enable backward compatibility.

If the deprecating styleOptions.bubbleImageHeight is set, it will propagate to min/max value and is backward compatible.

Specific Changes

  • Added styleOptions.bubbleImageMaxHeight and styleOptions.bubbleImageMinHeight, both defaults to 240
  • Deprecated styleOptions.bubbleImageHeight and changed default to undefined, which will honor min/max
  • Updated some VRT due to sub-pixel differences
  • I have added tests and executed them locally
  • I have updated CHANGELOG.md
  • I have updated documentation

Review Checklist

This section is for contributors to review your work.

  • Accessibility reviewed (tab order, content readability, alt text, color contrast)
  • Browser and platform compatibilities reviewed
  • CSS styles reviewed (minimal rules, no z-index)
  • Documents reviewed (docs, samples, live demo)
  • Internationalization reviewed (strings, unit formatting)
  • package.json and package-lock.json reviewed
  • Security reviewed (no data URIs, check for nonce leak)
  • Tests reviewed (coverage, legitimacy)

@compulim compulim marked this pull request as ready for review July 6, 2024 01:47
OEvgeny
OEvgeny previously approved these changes Jul 6, 2024
tdurnford
tdurnford previously approved these changes Jul 8, 2024
@compulim compulim dismissed stale reviews from tdurnford and OEvgeny via 89bc070 August 5, 2024 19:15
@compulim compulim merged commit 516e73e into main Aug 6, 2024
25 checks passed
@compulim compulim deleted the feat-variable-image-height branch August 6, 2024 06:00
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.

Use object-fit for <CroppedImage> so we don't need a fixed height
3 participants