Skip to content
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

fix: The page order doesn't work as expected in page setting page #13448

Merged
merged 5 commits into from
May 11, 2022
Merged

fix: The page order doesn't work as expected in page setting page #13448

merged 5 commits into from
May 11, 2022

Conversation

rahulramesha
Copy link
Contributor

@rahulramesha rahulramesha commented Apr 29, 2022 •
edited by github-actions bot
Loading

Description

This bug is caused because of using index as react keys while generating similiar siblings.

Fixes #12758
Fixes #9991

Type of change

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

How Has This Been Tested?

Manual UI testing

Checklist:

  • 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

Test coverage results 🧪

🔴 Total coverage has decreased 
// Code coverage diff between base branch:release and head branch: PageReorderFix
Status File % Stmts % Branch % Funcs % Lines
🔴 total 56.59 (0) 38.39 (-0.01) 35.84 (0) 56.83 (-0.01)
🔴 app/client/src/components/ads/DraggableList.tsx 35.87 (0) 15.71 (-0.47) 26.67 (0) 38.75 (0)
🔴 app/client/src/utils/autocomplete/TernServer.ts 52.71 (-0.23) 40.83 (-0.84) 36.21 (0) 56.74 (-0.25)

@vercel
Copy link

vercel bot commented Apr 29, 2022 •
edited
Loading

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

Name Status Preview Updated
appsmith ✅ Ready (Inspect) Visit Preview May 10, 2022 at 7:44AM (UTC)

@rahulramesha rahulramesha added UI Building Pod UI Builders Pod Issues that UI Builders face using appsmith labels Apr 29, 2022
@github-actions github-actions bot added Bug Something isn't working Low An issue that is neither critical nor breaks a user flow Needs Triaging Needs attention from maintainers to triage Page Management Issues related to configuring pages Release labels Apr 29, 2022
@github-actions
Copy link

Unable to find test scripts. Please add necessary tests to the PR.

4 similar comments
@github-actions
Copy link

Unable to find test scripts. Please add necessary tests to the PR.

@github-actions
Copy link

Unable to find test scripts. Please add necessary tests to the PR.

@github-actions
Copy link

Unable to find test scripts. Please add necessary tests to the PR.

@github-actions
Copy link

Unable to find test scripts. Please add necessary tests to the PR.

@github-actions github-actions bot added High This issue blocks a user from building or impacts a lot of users Production labels Apr 29, 2022
@github-actions
Copy link

Unable to find test scripts. Please add necessary tests to the PR.

@github-actions github-actions bot added Platform Pod potential-duplicate This label marks issues that are potential duplicates of already open issues Admin Settings Issues in admin settings pages Team Managers Pod Issues that team managers care about for the security and efficiency of their teams labels Apr 29, 2022
@github-actions
Copy link

Unable to find test scripts. Please add necessary tests to the PR.

@jsartisan
Copy link
Contributor

@rahulramesha Can you check if this is working fine in the widgets property pane where we use draggable list.

@rahulramesha
Copy link
Contributor Author

@jsartisan, yes those seemed to be working fine. Wanted to change those as well, but did not find a reliable ID that I can use as key. When I change the key to item[i].id table's column draggable list was not working as expected so I just let them be for now.

jsartisan
jsartisan previously approved these changes Apr 29, 2022
View reviewed changes
@github-actions
Copy link

github-actions bot commented May 7, 2022

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, 2022
@rahulramesha
Copy link
Contributor Author

/ok-to-test sha=bcc26aa

@rahulramesha rahulramesha removed the Stale label May 8, 2022
@github-actions
Copy link

github-actions bot commented May 8, 2022

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2290205840.
Workflow: Appsmith External Integration Test Workflow.
Commit: bcc26aa.
PR: 13448.

@github-actions
Copy link

github-actions bot commented May 8, 2022

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2290205840.
Commit: bcc26aa.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Median Mean SD.Sample SD.Population
SELECT_CATEGORY
scripting 912.89 2272.66 806.77 1953.08 734.85 912.89 1336.05 53.96 48.26
painting 7.55 12.16 4.67 4.32 11.37 7.55 8.01 45.69 40.82
rendering 346.64 329.4 340.77 309.79 304.71 329.4 326.26 5.68 5.08
BIND_TABLE_DATA
scripting 2232.6 2340.38 2248.74 2183.6 2268.13 2248.74 2254.69 2.54 2.27
painting 15.18 17.02 23.14 20.34 25.22 20.34 20.18 20.61 18.43
rendering 685.17 665.26 678.97 697.42 608 678.97 666.96 5.24 4.69
CLICK_ON_TABLE_ROW
scripting 2299.4 2263.12 2078.22 1894.29 1932.32 2078.22 2093.47 8.84 7.91
painting 23.31 22.09 22.72 15.51 19.51 22.09 20.63 15.56 13.91
rendering 314.84 328.03 297.83 295.19 300.41 300.41 307.26 4.52 4.04
UPDATE_POST_TITLE
scripting 3832.33 3461.41 3112.47 3218.97 3600.8 3461.41 3445.2 8.42 7.53
painting 18.49 18.18 14.23 18.18 23.92 18.18 18.6 18.60 16.61
rendering 455.35 430.61 385.74 400.45 413.39 413.39 417.11 6.48 5.79
OPEN_MODAL
scripting 1251.99 1371.78 1587.55 2900.64 1312.7 1371.78 1684.93 41.03 36.70
painting 10.1 15.03 17.21 9.46 11.85 11.85 12.73 26.00 23.25
rendering 438.46 461.24 523.95 429.27 428.13 438.46 456.21 8.80 7.87
CLOSE_MODAL
scripting 978.57 864.69 722.89 776.76 774.08 776.76 823.4 12.22 10.93
painting 6.14 5.47 5.12 5.31 5.27 5.31 5.46 7.33 6.59
rendering 420 389.97 370.39 351.2 366.31 370.39 379.57 6.98 6.24
SELECT_WIDGET_MENU_OPEN
scripting 1514 1540.58 1522.34 1521.06 1456.64 1521.06 1510.92 2.11 1.89
painting 4.95 5.26 12.45 4.95 5.1 5.1 6.54 50.61 45.26
rendering 499.12 501.66 509.08 494.17 497.78 499.12 500.36 1.11 1.00
SELECT_WIDGET_SELECT_OPTION
scripting 208.77 213.88 226.37 198.23 208.02 208.77 211.05 4.86 4.35
painting 2.73 2.67 3.04 2.45 2.9 2.73 2.76 8.33 7.25
rendering 16.5 15.98 16.07 15.8 16.22 16.07 16.11 1.61 1.49

1 similar comment
@github-actions
Copy link

github-actions bot commented May 8, 2022

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2290205840.
Commit: bcc26aa.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Median Mean SD.Sample SD.Population
SELECT_CATEGORY
scripting 912.89 2272.66 806.77 1953.08 734.85 912.89 1336.05 53.96 48.26
painting 7.55 12.16 4.67 4.32 11.37 7.55 8.01 45.69 40.82
rendering 346.64 329.4 340.77 309.79 304.71 329.4 326.26 5.68 5.08
BIND_TABLE_DATA
scripting 2232.6 2340.38 2248.74 2183.6 2268.13 2248.74 2254.69 2.54 2.27
painting 15.18 17.02 23.14 20.34 25.22 20.34 20.18 20.61 18.43
rendering 685.17 665.26 678.97 697.42 608 678.97 666.96 5.24 4.69
CLICK_ON_TABLE_ROW
scripting 2299.4 2263.12 2078.22 1894.29 1932.32 2078.22 2093.47 8.84 7.91
painting 23.31 22.09 22.72 15.51 19.51 22.09 20.63 15.56 13.91
rendering 314.84 328.03 297.83 295.19 300.41 300.41 307.26 4.52 4.04
UPDATE_POST_TITLE
scripting 3832.33 3461.41 3112.47 3218.97 3600.8 3461.41 3445.2 8.42 7.53
painting 18.49 18.18 14.23 18.18 23.92 18.18 18.6 18.60 16.61
rendering 455.35 430.61 385.74 400.45 413.39 413.39 417.11 6.48 5.79
OPEN_MODAL
scripting 1251.99 1371.78 1587.55 2900.64 1312.7 1371.78 1684.93 41.03 36.70
painting 10.1 15.03 17.21 9.46 11.85 11.85 12.73 26.00 23.25
rendering 438.46 461.24 523.95 429.27 428.13 438.46 456.21 8.80 7.87
CLOSE_MODAL
scripting 978.57 864.69 722.89 776.76 774.08 776.76 823.4 12.22 10.93
painting 6.14 5.47 5.12 5.31 5.27 5.31 5.46 7.33 6.59
rendering 420 389.97 370.39 351.2 366.31 370.39 379.57 6.98 6.24
SELECT_WIDGET_MENU_OPEN
scripting 1514 1540.58 1522.34 1521.06 1456.64 1521.06 1510.92 2.11 1.89
painting 4.95 5.26 12.45 4.95 5.1 5.1 6.54 50.61 45.26
rendering 499.12 501.66 509.08 494.17 497.78 499.12 500.36 1.11 1.00
SELECT_WIDGET_SELECT_OPTION
scripting 208.77 213.88 226.37 198.23 208.02 208.77 211.05 4.86 4.35
painting 2.73 2.67 3.04 2.45 2.9 2.73 2.76 8.33 7.25
rendering 16.5 15.98 16.07 15.8 16.22 16.07 16.11 1.61 1.49

@rahulramesha rahulramesha mentioned this pull request May 9, 2022
Merged
7 tasks
@rahulramesha
Copy link
Contributor Author

/ok-to-test sha=3874515

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2299043974.
Workflow: Appsmith External Integration Test Workflow.
Commit: 3874515.
PR: 13448.

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2299043974.
Commit: 3874515.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Median Mean SD.Sample SD.Population
SELECT_CATEGORY
scripting 916.87 960.47 796.39 959.05 807.94 916.87 888.14 9.07 8.11
painting 12.27 5.35 3.9 5.78 10.36 5.78 7.53 47.68 42.63
rendering 326.87 427.63 299.57 320.73 333.15 326.87 341.59 14.56 13.02
BIND_TABLE_DATA
scripting 2307.33 2149.48 2130.77 2850.7 2304.34 2304.34 2348.52 12.47 11.15
painting 30.7 25.54 27.29 31.29 22.92 27.29 27.55 12.78 11.40
rendering 879.97 998.46 942.67 861.84 622.28 879.97 861.04 16.72 14.96
CLICK_ON_TABLE_ROW
scripting 3281.35 3775.21 3987.35 3845.29 2014.19 3775.21 3380.68 23.92 21.40
painting 30.33 40.94 53.62 38.94 16.25 38.94 36.02 38.42 34.37
rendering 513.23 473.58 765.38 680.03 287.49 513.23 543.94 34.30 30.68
UPDATE_POST_TITLE
scripting 4457.59 3950.07 4179.7 3828.32 3142.94 3950.07 3911.72 12.58 11.26
painting 21.71 24.24 18.92 32.28 13.5 21.71 22.13 31.36 28.02
rendering 576.67 432.41 460.73 522.4 388.03 460.73 476.05 15.63 13.98
OPEN_MODAL
scripting 4207.1 1888.64 1931.37 1753.26 1375.33 1888.64 2231.14 50.47 45.14
painting 14.95 19.27 14.35 17.26 12.15 14.95 15.6 17.56 15.71
rendering 596.28 645.38 630.81 569.57 454.32 596.28 579.27 13.10 11.71
CLOSE_MODAL
scripting 1074.93 2996.29 1087.78 937.74 716.44 1074.93 1362.64 67.91 60.74
painting 7.39 15.61 5.98 6.67 7.54 7.39 8.64 45.72 40.86
rendering 470.46 454.5 430.02 544.22 347.58 454.5 449.36 15.82 14.15
SELECT_WIDGET_MENU_OPEN
scripting 1597.33 1519.14 1494.95 1446.52 1560.29 1519.14 1523.65 3.82 3.42
painting 7.36 8.56 6.21 12.51 9.98 8.56 8.92 27.47 24.55
rendering 503 522.23 493.19 490.52 498.58 498.58 501.5 2.50 2.24
SELECT_WIDGET_SELECT_OPTION
scripting 223.19 247.83 269.08 218.22 211.36 223.19 233.94 10.25 9.17
painting 3.36 2.89 3.09 2.6 2.51 2.89 2.89 12.11 10.73
rendering 16.58 15.54 15.4 16.08 14.76 15.54 15.67 4.40 3.96

1 similar comment
@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2299043974.
Commit: 3874515.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Median Mean SD.Sample SD.Population
SELECT_CATEGORY
scripting 916.87 960.47 796.39 959.05 807.94 916.87 888.14 9.07 8.11
painting 12.27 5.35 3.9 5.78 10.36 5.78 7.53 47.68 42.63
rendering 326.87 427.63 299.57 320.73 333.15 326.87 341.59 14.56 13.02
BIND_TABLE_DATA
scripting 2307.33 2149.48 2130.77 2850.7 2304.34 2304.34 2348.52 12.47 11.15
painting 30.7 25.54 27.29 31.29 22.92 27.29 27.55 12.78 11.40
rendering 879.97 998.46 942.67 861.84 622.28 879.97 861.04 16.72 14.96
CLICK_ON_TABLE_ROW
scripting 3281.35 3775.21 3987.35 3845.29 2014.19 3775.21 3380.68 23.92 21.40
painting 30.33 40.94 53.62 38.94 16.25 38.94 36.02 38.42 34.37
rendering 513.23 473.58 765.38 680.03 287.49 513.23 543.94 34.30 30.68
UPDATE_POST_TITLE
scripting 4457.59 3950.07 4179.7 3828.32 3142.94 3950.07 3911.72 12.58 11.26
painting 21.71 24.24 18.92 32.28 13.5 21.71 22.13 31.36 28.02
rendering 576.67 432.41 460.73 522.4 388.03 460.73 476.05 15.63 13.98
OPEN_MODAL
scripting 4207.1 1888.64 1931.37 1753.26 1375.33 1888.64 2231.14 50.47 45.14
painting 14.95 19.27 14.35 17.26 12.15 14.95 15.6 17.56 15.71
rendering 596.28 645.38 630.81 569.57 454.32 596.28 579.27 13.10 11.71
CLOSE_MODAL
scripting 1074.93 2996.29 1087.78 937.74 716.44 1074.93 1362.64 67.91 60.74
painting 7.39 15.61 5.98 6.67 7.54 7.39 8.64 45.72 40.86
rendering 470.46 454.5 430.02 544.22 347.58 454.5 449.36 15.82 14.15
SELECT_WIDGET_MENU_OPEN
scripting 1597.33 1519.14 1494.95 1446.52 1560.29 1519.14 1523.65 3.82 3.42
painting 7.36 8.56 6.21 12.51 9.98 8.56 8.92 27.47 24.55
rendering 503 522.23 493.19 490.52 498.58 498.58 501.5 2.50 2.24
SELECT_WIDGET_SELECT_OPTION
scripting 223.19 247.83 269.08 218.22 211.36 223.19 233.94 10.25 9.17
painting 3.36 2.89 3.09 2.6 2.51 2.89 2.89 12.11 10.73
rendering 16.58 15.54 15.4 16.08 14.76 15.54 15.67 4.40 3.96

@rahulramesha rahulramesha enabled auto-merge (squash) May 10, 2022 15:56
@rahulramesha rahulramesha merged commit 3923204 into release May 11, 2022
@rahulramesha rahulramesha deleted the PageReorderFix branch May 11, 2022 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsartisan jsartisan jsartisan approved these changes

Assignees
No one assigned
Labels
Admin Settings Issues in admin settings pages Bug Something isn't working High This issue blocks a user from building or impacts a lot of users Low An issue that is neither critical nor breaks a user flow Needs Triaging Needs attention from maintainers to triage Page Management Issues related to configuring pages potential-duplicate This label marks issues that are potential duplicates of already open issues Production Release Team Managers Pod Issues that team managers care about for the security and efficiency of their teams UI Builders Pod Issues that UI Builders face using appsmith
Projects
None yet
Milestone
No milestone
2 participants
@rahulramesha @jsartisan