Skip to content

fix: add borderWidth to column space calculation #22738

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.

Sign up for GitHub

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

Jump to bottom
Open
prsidhu wants to merge 6 commits into
base: release
Choose a base branch
from
Open

fix: add borderWidth to column space calculation #22738

prsidhu wants to merge 6 commits into from

Conversation

@prsidhu
Copy link
Contributor

@prsidhu prsidhu commented Apr 25, 2023

Description

Issue: increasing border width of containers cuts off widgets and highlights along the edges.
Cause: Column space calculation doesn't account for border widths.

Fix: To account for parent's border width in canvas' column space calculation, the value is passed as a new prop parentBorderWidth. Discount the value from the canvas width to rectify the calclulation

Add a TL;DR when description is extra long (helps content team)

Fixes # (issue)

  1. [Bug]: [Column Space] Consider container border width in column space calculation #22680

Media

Screen.Recording.2023-04-25.at.4.52.20.PM.mov

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Manual

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
  • 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
  • PR is being merged under a feature flag

@prsidhu prsidhu mentioned this pull request Apr 25, 2023
Closed
@github-actions github-actions bot added the Bug Something isn't working label Apr 25, 2023
@prsidhu
Copy link
Contributor Author

prsidhu commented Apr 25, 2023

/build-deploy-preview env=release

@prsidhu
Copy link
Contributor Author

prsidhu commented Apr 25, 2023

/ok-to-test

@github-actions
Copy link

github-actions bot commented Apr 25, 2023

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/4802110693.
Workflow: On demand build Docker image and deploy preview.
skip-tests: ``.
env: release.
PR: 22738.
recreate: .

@github-actions
Copy link

github-actions bot commented Apr 25, 2023

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

@prsidhu prsidhu self-assigned this Apr 25, 2023
@github-actions
Copy link

github-actions bot commented Apr 25, 2023

Deploy-Preview-URL: https://appsmith-fsqsaagve-get-appsmith.vercel.app

@prsidhu
Copy link
Contributor Author

prsidhu commented Apr 26, 2023

/ok-to-test

@github-actions
Copy link

github-actions bot commented Apr 26, 2023

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

@github-actions
Copy link

github-actions bot commented Apr 26, 2023

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4804481283.
Commit: ``.
The following are new failures, please fix them before merging the PR:

  1. cypress/integration/Regression_TestSuite/ServerSideTests/Datasources/Redis_Basic_Spec.ts

To know the list of identified flaky tests - Refer here

@prsidhu
Copy link
Contributor Author

prsidhu commented Apr 28, 2023

/ok-to-test

@github-actions
Copy link

github-actions bot commented Apr 28, 2023

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

marks0351
marks0351 previously approved these changes Apr 28, 2023
View reviewed changes
@github-actions
Copy link

github-actions bot commented Apr 28, 2023

Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4832276329.
Commit: ``.
The following are new failures, please fix them before merging the PR:


    To know the list of identified flaky tests - Refer here

    1 similar comment
    @github-actions
    Copy link

    github-actions bot commented Apr 28, 2023

    Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4832276329.
    Commit: ``.
    The following are new failures, please fix them before merging the PR:


      To know the list of identified flaky tests - Refer here

      @prsidhu
      Copy link
      Contributor Author

      prsidhu commented Apr 28, 2023

      /ok-to-test

      @github-actions
      Copy link

      github-actions bot commented Apr 28, 2023

      Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4832276329.
      Commit: ``.
      The following are new failures, please fix them before merging the PR:


        To know the list of identified flaky tests - Refer here

        @github-actions
        Copy link

        github-actions bot commented Apr 28, 2023

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

        @github-actions
        Copy link

        github-actions bot commented Apr 28, 2023

        Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4835140259.
        Commit: ``.
        The following are new failures, please fix them before merging the PR:

        1. cypress/integration/Regression_TestSuite/ClientSideTests/Git/GitWithJSLibrary/GitwithCustomJSLibrary_spec.js

        2. cypress/integration/Regression_TestSuite/ServerSideTests/ApiTests/API_Search_spec.js
        To know the list of identified flaky tests - Refer here

        @github-actions
        Copy link

        github-actions bot commented Apr 29, 2023

        Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4835140259.
        Commit: ``.
        All cypress tests have passed 🎉

        2 similar comments
        @github-actions
        Copy link

        github-actions bot commented Apr 29, 2023

        Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4835140259.
        Commit: ``.
        All cypress tests have passed 🎉

        @github-actions
        Copy link

        github-actions bot commented Apr 29, 2023

        Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4835140259.
        Commit: ``.
        All cypress tests have passed 🎉

        @github-actions
        Copy link

        github-actions bot commented Apr 29, 2023

        Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4835140259.
        Commit: ``.
        All cypress tests have passed 🎉

        2 similar comments
        @github-actions
        Copy link

        github-actions bot commented Apr 29, 2023

        Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4835140259.
        Commit: ``.
        All cypress tests have passed 🎉

        @github-actions
        Copy link

        github-actions bot commented Apr 29, 2023

        Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/4835140259.
        Commit: ``.
        All cypress tests have passed 🎉

        @github-actions
        Copy link

        github-actions bot commented May 7, 2023

        This PR has not seen activitiy for a while. It will be closed in 7 days unless further activity is detected.

        @github-actions github-actions bot added the Stale label May 7, 2023
        @prsidhu
        Copy link
        Contributor Author

        prsidhu commented May 15, 2023

        /ok-to-test

        @github-actions
        Copy link

        github-actions bot commented May 15, 2023

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

        @github-actions github-actions bot removed the Stale label May 28, 2023
        Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

        Reviewers

        1 more reviewer

        @marks0351 marks0351 marks0351 left review comments

        Reviewers whose approvals may not affect merge requirements

        At least 1 approving review is required to merge this pull request.

        Assignees

        @prsidhu prsidhu

        Labels

        Bug Something isn't working

        Projects

        None yet

        Milestone

        No milestone

        Development

        Successfully merging this pull request may close these issues.

        4 participants

        @prsidhu @marks0351 @Nikhil-Nandagopal @KelvinOm