Skip to content

Conversation

buckett
Copy link
Member

@buckett buckett commented Jul 16, 2025

When limiting the height (for example because we are displaying a modal) we don't force the new size to be sent through the postMessage, but instead rely on the content changing to cause an update. But it's not always true that the content will changes (and trigger the mutation observer) so instead we always force a resize when changing the limit.

This also updates the component so that it only pushes a resize if it's different to the last size we had.

When limiting the height (for example because we are displaying a modal) we don't force the new size to be sent through the postMessage, but instead rely on the content changing to cause an update. But it's not always true that the content will changes (and trigger the mutation observer) so instead we always force a resize when changing the limit.

This also updates the component so that it only pushes a resize if it's different to the last size we had.
Copy link

Copy link

@Copilot 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 enhances the LtiHeightLimit component to always trigger a resize when the height limit is updated, while preventing redundant resize messages if the height hasn't actually changed.

  • Track the last sent height to skip duplicate postMessage calls.
  • Always call resize() after updating the limit state.
  • Add debug logging around limit updates and resize operations.

// scroll bar showing when the height being rounded down. Canvas appears to allow a float (it works), but
// we round up (ceil) so that we're complying with the API documentation in case they change in the future.
Math.ceil(document.documentElement.getBoundingClientRect().height)
if (height === this.lastHeight) {
Copy link
Preview

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

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

Since limit changes should always force a resize, this guard can block the forced update when the new height equals the previous height. Consider adding a flag to bypass this check for limit-triggered resizes or clear lastHeight before calling resize.

Copilot uses AI. Check for mistakes.

set: (limit) => {
this.setState({ limit })
this.logDebug(`Updating limit: ${limit}`)
this.setState({ limit }, () => this.resize())
Copy link
Preview

Copilot AI Jul 16, 2025

Choose a reason for hiding this comment

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

To guarantee the forced resize always sends a message, reset this.lastHeight (e.g., this.lastHeight = -1) before invoking this.resize() in the limit setter callback.

Suggested change
this.setState({ limit }, () => this.resize())
this.setState({ limit }, () => {
this.lastHeight = -1;
this.resize();
})

Copilot uses AI. Check for mistakes.

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.

1 participant