Skip to content

Conversation

@riodeuno
Copy link
Contributor

@riodeuno riodeuno commented Dec 20, 2022

Description

This PR adds one of the promised updates to the Auto-height feature---to see containers change height as we drag and drop widgets within them instead of after dropping these widgets.

  • Display Auto-height updates during drag and resize, instead of after. See video below.

  • Include scrollbars within borders of the container like widgets such that the borders are not overridden by scrollbars and auto height considers border thickness for computations

  • Make Container widgets leaner by reducing the number of DOM nodes which make up a container-like widget. This reduces the load on the browser, so that it doesn't have to render as many DOM nodes as before. So, this is improving rendering performance, however, it may not be noticeable, as it is in milliseconds. We haven't registered any metrics for this, as this falls in the best practices category instead of the performance improvement category.

Fixes #19216
Fixes #19578
Fixes #19579
Fixes #20421

Media

Auto.height.with.instant.feedback.mov

Type of change

  • This changes the way auto height applies updates, particularly w.r.t borders in container like widgets.
  • This change will result in a different experience, which is an improvement on the existing experience.

How Has This Been Tested?

  • Manual: Scenarios will be tested manually. Please see Test Plan for more information.
  • Jest: Functions added have unit tests
    This involves drag and drop and will not have cypress tests.

Test Plan

https://github.com/appsmithorg/TestSmith/issues/2145

Issues raised during DP testing

#19082 (comment)
#19082 (comment)
#19082 (comment)

Checklist:

Dev activity

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • [NA] I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • [NA] PR is being merged under a feature flag

QA activity:

  • Test plan has been approved by relevant developers
  • Test plan has been peer reviewed by QA
  • Cypress test cases have been added and approved by either SDET or manual QA
  • Organized project review call with relevant stakeholders after Round 1/2 of QA
  • Added Test Plan Approved label after reveiwing all Cypress test

@riodeuno riodeuno self-assigned this Dec 20, 2022
@vercel
Copy link

vercel bot commented Dec 20, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
appsmith ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Feb 2, 2023 at 4:20PM (UTC)

@SatishGandham
Copy link
Contributor

/perf-test
Testing performance infra, ignore.

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3764339506.
Workflow: Performance Tests Workflow.
Commit: ``.
PR: 19082.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=19082&runId=3764339506_1

…t and Tabs widget, as well as the canvas default height
@riodeuno riodeuno added Enhancement New feature or request Modal Widget Container Widget Container widget Drag & Drop Issues related to the drag & drop experience Reflow & Resize All issues related to reflow and resize experience Auto Height Issues related to dynamic height of widgets Canvas / Grid Issues related to the canvas labels Jan 5, 2023
@github-actions github-actions bot added Bug Something isn't working Needs Triaging Needs attention from maintainers to triage UI Building Product Issues related to the UI Building experience labels Jan 9, 2023
@github-actions
Copy link

github-actions bot commented Feb 2, 2023

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4075423889.
Workflow: Performance Tests Workflow.
Commit: ``.
PR: 19082.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=19082&runId=4075423889_1

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

The following are new failures, please fix them before merging the PR

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

The following are new failures, please fix them before merging the PR cypress/integration/Regression_TestSuite/Application/PgAdmin_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/RepoLimitExceededErrorModal_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/OtherUIFeatures/Logs_spec.ts
cypress/integration/Regression_TestSuite/ClientSideTests/Templates/Fork_Template_To_App_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/VisualTests/JSEditorComment_spec.js

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

The following are new failures, please fix them before merging the PR

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

The following are new failures, please fix them before merging the PR cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/RepoLimitExceededErrorModal_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/RepoLimitExceededErrorModal_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/OtherUIFeatures/Logs_spec.ts
cypress/integration/Regression_TestSuite/ClientSideTests/OtherUIFeatures/Logs_spec.ts
cypress/integration/Regression_TestSuite/ClientSideTests/Templates/Fork_Template_To_App_spec.js

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

The following are new failures, please fix them before merging the PR

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

The following are new failures, please fix them before merging the PR cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/RepoLimitExceededErrorModal_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/RepoLimitExceededErrorModal_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/OtherUIFeatures/Logs_spec.ts
cypress/integration/Regression_TestSuite/ClientSideTests/OtherUIFeatures/Logs_spec.ts
cypress/integration/Regression_TestSuite/ClientSideTests/OtherUIFeatures/Logs_spec.ts

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

The following are new failures, please fix them before merging the PR

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

The following are new failures, please fix them before merging the PR cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/RepoLimitExceededErrorModal_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitSync/RepoLimitExceededErrorModal_spec.js
cypress/integration/Regression_TestSuite/ClientSideTests/OtherUIFeatures/Logs_spec.ts
cypress/integration/Regression_TestSuite/ClientSideTests/OtherUIFeatures/Logs_spec.ts
cypress/integration/Regression_TestSuite/ClientSideTests/OtherUIFeatures/Logs_spec.ts
cypress/integration/Regression_TestSuite/ClientSideTests/VisualTests/JSEditorComment_spec.js

@Aishwarya-U-R
Copy link
Contributor

/ok-to-test sha=6b800ad

@github-actions
Copy link

github-actions bot commented Feb 2, 2023

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/4076423470.
Workflow: Appsmith External Integration Test Workflow.
Commit: 6b800ad.
PR: 19082.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=19082&runId=4076423470_1

@riodeuno riodeuno merged commit 20de520 into release Feb 3, 2023
@riodeuno riodeuno deleted the feat/auto-height-instant-update branch February 3, 2023 05:47
@github-actions github-actions bot added Sentry Issues reported from Sentry errors and removed Bug Something isn't working labels Feb 7, 2023
@kamakshibhat-appsmith
Copy link

With this PR , do we expect fix for the issue where scrollbars were coming in deploy mode when border width was increased for container type(auto height) widgets ? @riodeuno

@github-actions github-actions bot added Bug Something isn't working and removed Bug Something isn't working labels Feb 12, 2023
@riodeuno
Copy link
Contributor Author

@kamakshibhat-appsmith Yes, this PR should have fixed that. I can replicate this on release right now. Let me look into this.

@github-actions github-actions bot added Bug Something isn't working and removed Bug Something isn't working labels Feb 16, 2023
@marks0351 marks0351 restored the feat/auto-height-instant-update branch March 28, 2023 14:17
@sharat87 sharat87 deleted the feat/auto-height-instant-update branch November 23, 2024 15:22
ankitakinger added a commit that referenced this pull request Aug 20, 2025
…xed height container (#41178)

## Description

Removing a line of code to fix extra space issue on Page with Fixed
height container once switched from a Page with Auto height container.
Also, manually tested out all issues from
[#19082](#19082) to confirm
nothing else breaks from the time these lines were added in the code.

Fixes [#41180](#41180)

## Automation

/ok-to-test tags="@tag.All"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/17043252133>
> Commit: c8bde12
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=17043252133&attempt=2"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.All`
> Spec:
> <hr>Mon, 18 Aug 2025 17:04:42 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **Bug Fixes**
* Improved main container auto-height calculation to eliminate
unintended extra spacing, resulting in more accurate, content-driven
sizing.

* **Chores**
* Added diagnostic logging around main container size computation in
view mode to aid troubleshooting (no functional impact).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Auto Height Issues related to dynamic height of widgets Canvas / Grid Issues related to the canvas Container Widget Container widget Drag & Drop Issues related to the drag & drop experience Enhancement New feature or request Medium Issues that frustrate users due to poor UX Modal Widget Needs Triaging Needs attention from maintainers to triage QA Needs QA attention Reflow & Resize All issues related to reflow and resize experience Sentry Issues reported from Sentry errors Test Plan Approved Manual/Cypress tests covers changes made on the PR. Else, add skip-testPlan label if not applicable UI Building Product Issues related to the UI Building experience UI Improvement

Projects

None yet

6 participants