Modal Adjust to Input Size

[galovdev]

Description

The modal now resizes like the description input every time the input size changes.

Issue addressed

Addresses #570

Before/after screenshots

Before:

After:

Grabacion.de.pantalla.2024-03-25.a.la.s.3.32.35.p.m.mov

Changelog

• #590
  • Description: Modal now resizes dynamically to adjust size based on content changes.
  • Products impact: bugfix.
  • Addresses: KModal does not shrink when the content has a smaller height #570
  • Components: KModal.
  • Breaking: No
  • Impacts a11y: -
  • Guidance: Consumers need to ensure the modal content is wrapped correctly for the resizing logic to work effectively. No additional steps required for integration.

Steps to test

1. Go to studio
2. Studio channels
3. Edit title and description

Testing checklist

• [ X] Contributor has fully tested the PR manually
• [ X] If there are any front-end changes, before/after screenshots are included
• Critical and brittle code paths are covered by unit tests
• The change is described in the changelog section above

After review

• The changelog item has been pasted to the CHANGELOG.md

[AlexVelezLl]

Hi! @galovdev This is looking better! But we still need to find a way how to update the height of the content, I will check with the team for further Ideas. Also, I see there are some linting errors, you can fix them with yarn run lint-fix and also we need to update the CHANGELOG.md file with the update of this PR.

[AlexVelezLl]

The reason we should not add listeners to the inputs is that inputs are not just the ones that will grow or shrink, this can be anything that are conditionally rendered within the modal, so this solution will not work for modals that have a form that could (conditionally) have warnings and errors, just to give an example.

Check this video I recorded running this PR:

Compartir.pantalla.-.2024-03-26.08_15_43.mp4

This is because this solution is not general for all use cases. Also, we should try to avoid setting the content height to auto, as this could cause too many reflows.

We need to think a way to track the actual height of the content and update accordingly only when there are more than 8px of difference.

[AlexVelezLl]

Hi @galovdev I talked with the team, and we are going to take the approach of keep it simple, and better set an auto height, and try to avoid the pixels toggling like the one on KTexbox hover. You are free to try to fix the KTextbox pixel toggling on hover if you want, but for now just setting the content height to auto works fine.

Also, we will need to retarget the PR to the develop branch. You can find how to retarget PRs here.

[galovdev]

We should avoid setting the height directly to the content element, when we set the contentHeight attribute in line 298 we already are setting the height of the template.

Give me till monday to make the changes and re'target the branch, ill be out this weekend.

[AlexVelezLl]

Hi! @galovdev How are you? Have you been able to advance in the PR? I'm not sure if maybe someway I misdirected you, but we are expecting in this PR that we clean up the JS code related to the this.contentHeight calculation, so it will be better to delete this line, this method, and this logic, and any code that became unused because of the changes.

Also we will need to retarget the PR to the develop branch. So first you can rebase your branch onto the develop branch as indicated here, and after that you can change the base branch here in github.

Let me know if you have any question :).

[AlexVelezLl]

It looks like we accidentally deleted this line and we need to bring it back.

[AlexVelezLl]

Hi @galovdev! How are you? Have you been able to advance in the PR? Let us know if there is something we can help with.

[AlexVelezLl]

Thank you @galovdev! Code looks good, and QA has confirmed that there are no regressions. Its good to go! 🎉.