Skip to content

Conversation

sebastianchristopher
Copy link
Contributor

The page doesn't rerender when limit changes, so modals aren't positioned accordingly

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 addresses a rendering issue where modals are not positioned correctly when the height limit state changes from false to true. The fix ensures that the component recalculates its height when the limit becomes active.

  • Adds componentDidUpdate lifecycle method to detect when limit state changes from false to true
  • Triggers a resize operation when the limit becomes active to reposition modals correctly

}

componentDidUpdate(prevProps, prevState) {
if (!prevState.limit && this.state.limit) {
Copy link
Preview

Copilot AI Aug 6, 2025

Choose a reason for hiding this comment

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

Consider also handling the case when limit changes from true to false, as this state change might also require repositioning of modals for consistency.

Suggested change
if (!prevState.limit && this.state.limit) {
if (prevState.limit !== this.state.limit) {

Copilot uses AI. Check for mistakes.

Copy link
Member

Choose a reason for hiding this comment

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

I think the co-pilot change makes sense as we should be re-sizing any time the limit state is changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I completely missed that you proposed something very similar in #59 a few weeks ago. Oh well!

Copy link
Member

Choose a reason for hiding this comment

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

I forgot as well and I wrote it!

Copy link

sonarqubecloud bot commented Aug 6, 2025

Copy link
Contributor

@nicholaswilson100 nicholaswilson100 left a comment

Choose a reason for hiding this comment

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

lgtm

@sebastianchristopher sebastianchristopher merged commit dfd0289 into master Aug 11, 2025
6 checks passed
@sebastianchristopher sebastianchristopher deleted the AB#92895 branch August 11, 2025 10:45
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