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.

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

fix: Dropdown ui fixes - hover and highlight color, keyboard navigation #16718

Merged
merged 16 commits into from
Sep 29, 2022

Conversation

albinAppsmith
Copy link
Collaborator

@albinAppsmith albinAppsmith commented Sep 13, 2022

Description

This new version of design system package includes the fix for Dropdown v1 UI inconsistencies fix and search auto close fix.

Fixes #16388, #16756

Type of change

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

How Has This Been Tested?

  • Tested locally

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

@vercel
Copy link

vercel bot commented Sep 13, 2022

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

Name Status Preview Updated
appsmith ✅ Ready (Inspect) Visit Preview Sep 27, 2022 at 8:09PM (UTC)

@github-actions github-actions bot added Bug Something isn't working Design System Pod Appsmith design system related issues Medium Issues that frustrate users due to poor UX Production labels Sep 13, 2022
@github-actions
Copy link

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

@albinAppsmith
Copy link
Collaborator Author

/ok-to-test sha=4624f3e

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3044379378.
Workflow: Appsmith External Integration Test Workflow.
Commit: 4624f3e.
PR: 16718.

@albinAppsmith
Copy link
Collaborator Author

/ok-to-test sha=c750007

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3048174063.
Workflow: Appsmith External Integration Test Workflow.
Commit: c750007.
PR: 16718.

@github-actions
Copy link

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

Click to view performance test results

| | Run 1 (ms)| Run 2 (ms)| Run 3 (ms)| Run 4 (ms)| Run 5 (ms)| Minimum (ms)| Median (ms)| Mean (ms)| Range (%) | SD.Sample (%) | SD.Population (%)|
| ------------- | ------------- | ------------- | ------------- | ------------- | ------------- | ------------- | ------------- | ------------- | ------------- | ------------- |
SELECT_CATEGORY| | | | | | | | | | |
| scripting | 324.85 | 1023.58 | 306.19 | 322.98 | 319.84 | 306.19| 322.98| 459.49 | 156.13| 68.65 | 61.40|
| painting | 3.16 | 4.87 | 4.68 | 3.47 | 3.59 | 3.16| 3.59| 3.95 | 43.29| 19.49 | 17.47|
| rendering | 101.88 | 105.65 | 103.7 | 106.25 | 103.96 | 101.88| 103.96| 104.29 | 4.19| 1.66 | 1.49|
BIND_TABLE_DATA| | | | | | | | | | |
| scripting | 933.39 | 993.45 | 995.46 | 967.49 | 1020.51 | 933.39| 993.45| 982.06 | 8.87| 3.37 | 3.01|
| painting | 13.41 | 13.02 | 22.49 | 18.29 | 25.28 | 13.02| 18.29| 18.5 | 66.27| 29.35 | 26.27|
| rendering | 788.45 | 797.89 | 803.19 | 787.58 | 796.75 | 787.58| 796.75| 794.77 | 1.96| 0.84 | 0.75|
CLICK_ON_TABLE_ROW| | | | | | | | | | |
| scripting | 1025.95 | 793.62 | 838.12 | 859.19 | 860.12 | 793.62| 859.19| 875.4 | 26.54| 10.09 | 9.03|
| painting | 8.94 | 8.87 | 8.74 | 9.39 | 10.31 | 8.74| 8.94| 9.25 | 16.97| 6.92 | 6.16|
| rendering | 320.64 | 286.64 | 300.22 | 310.47 | 296.32 | 286.64| 300.22| 302.86 | 11.23| 4.33 | 3.87|
UPDATE_POST_TITLE| | | | | | | | | | |
| scripting | 1838.26 | 1182.88 | 1196.38 | 1177.64 | 1164.9 | 1164.9| 1182.88| 1312.01 | 51.32| 22.44 | 20.07|
| painting | 15.84 | 12.52 | 13.72 | 12.04 | 13.22 | 12.04| 13.22| 13.47 | 28.21| 10.91 | 9.80|
| rendering | 598.83 | 579.1 | 611.33 | 579.1 | 575.29 | 575.29| 579.1| 588.73 | 6.12| 2.66 | 2.38|
OPEN_MODAL| | | | | | | | | | |
| scripting | 475.68 | 485.93 | 471.45 | 480.39 | 480.51 | 471.45| 480.39| 478.79 | 3.02| 1.14 | 1.02|
| painting | 12.67 | 13.1 | 12.16 | 10.47 | 15.78 | 10.47| 12.67| 12.84 | 41.36| 14.95 | 13.40|
| rendering | 1158.85 | 1175.71 | 1179.68 | 1195.35 | 1150.6 | 1150.6| 1175.71| 1172.04 | 3.82| 1.51 | 1.35|
CLOSE_MODAL| | | | | | | | | | |
| scripting | 254.39 | 251.16 | 249.44 | 269.97 | 252.2 | 249.44| 252.2| 255.43 | 8.04| 3.26 | 2.91|
| painting | 4.47 | 5.53 | 7.52 | 5.1 | 5.04 | 4.47| 5.1| 5.53 | 55.15| 21.16 | 18.99|
| rendering | 867.87 | 888.13 | 895.66 | 889.87 | 888.01 | 867.87| 888.13| 885.91 | 3.14| 1.19 | 1.07|
SELECT_WIDGET_MENU_OPEN| | | | | | | | | | |
| scripting | 908.74 | 900.87 | 926.61 | 895.25 | 907.35 | 895.25| 907.35| 907.76 | 3.45| 1.30 | 1.17|
| painting | 5.06 | 11.96 | 5.35 | 9.24 | 6.89 | 5.06| 6.89| 7.7 | 89.61| 37.66 | 33.64|
| rendering | 647.82 | 641.58 | 646.35 | 635.69 | 642.84 | 635.69| 642.84| 642.86 | 1.89| 0.74 | 0.66|
SELECT_WIDGET_SELECT_OPTION| | | | | | | | | | |
| scripting | 161.55 | 152.1 | 161.7 | 231.74 | 154.33 | 152.1| 161.55| 172.28 | 46.23| 19.45 | 17.40|
| painting | 6.43 | 3.7 | 2.77 | 3.74 | 6.22 | 2.77| 3.74| 4.57 | 80.09| 36.11 | 32.17|
| rendering | 312.83 | 308.23 | 299.45 | 296.84 | 321.81 | 296.84| 308.23| 307.83 | 8.11| 3.29 | 2.95|

@albinAppsmith
Copy link
Collaborator Author

/ok-to-test sha=373e16f

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3050469232.
Workflow: Appsmith External Integration Test Workflow.
Commit: 373e16f.
PR: 16718.

ankitakinger
ankitakinger previously approved these changes Sep 14, 2022
@shadabbuchh
Copy link

shadabbuchh commented Sep 14, 2022

Noticed a couple of issues in this PR:

  1. On the first try if we press space after entering any text in the dropdown, the space does not reflect in the search text.
  2. If we select any option and then try to enter something in the search text and then press space, the dropdown closes.

https://www.loom.com/share/faf074f358e14f2d90b3d7cb1544b294

  1. @albinAppsmith also once an option is selected, the checkmark next to it appears in orange colour whereas as per the figma doc it should be black in colour. Can we please confirm what colour are we going forward with?
    Screenshot 2022-09-14 at 4.43.08 PM.png

@albinAppsmith
Copy link
Collaborator Author

/ok-to-test sha=fdb1c8a

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3065776266.
Workflow: Appsmith External Integration Test Workflow.
Commit: fdb1c8a.
PR: 16718.

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3108917779.
Workflow: Appsmith External Integration Test Workflow.
Commit: 254a59c.
PR: 16718.

@github-actions
Copy link

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

Click to view performance test results

Run 1 (ms) Run 2 (ms) Run 3 (ms) Run 4 (ms) Run 5 (ms) Minimum (ms) Median (ms) Mean (ms) Range (%) SD.Sample (%) SD.Population (%)
SELECT_CATEGORY
scripting 335.87 425.36 339.11 344.01 347.61 335.87 344.01 358.39 24.97 10.52 9.41
painting 6.27 6.76 2.97 3.16 3.65 2.97 3.65 4.56 83.11 39.69 35.53
rendering 101.38 110.64 100.7 105.13 103.21 100.7 103.21 104.21 9.54 3.82 3.42
BIND_TABLE_DATA
scripting 1075.01 1154.38 1067.85 1127.61 1066.76 1066.76 1075.01 1098.32 7.98 3.66 3.28
painting 16.7 24.46 18.97 19.71 17.84 16.7 18.97 19.54 39.71 15.25 13.66
rendering 793.38 851.93 794.71 817.29 800.43 793.38 800.43 811.55 7.21 3.02 2.70
CLICK_ON_TABLE_ROW
scripting 802.96 917.54 912.45 905.02 892.94 802.96 905.02 886.18 12.93 5.35 4.79
painting 9.27 8.99 10.25 19.46 12.79 8.99 10.25 12.15 86.17 35.80 32.02
rendering 290.99 301.36 296.08 294.98 301.62 290.99 296.08 297.01 3.58 1.52 1.36
UPDATE_POST_TITLE
scripting 1447.07 1373.06 1472.77 1399.01 1391.45 1373.06 1399.01 1416.67 7.04 2.94 2.63
painting 15.24 13.28 13.36 13.82 20.16 13.28 13.82 15.17 45.35 19.12 17.07
rendering 600.34 613.16 588.59 596.45 592.45 588.59 596.45 598.2 4.11 1.58 1.41
OPEN_MODAL
scripting 490.35 441.49 446 416.03 442.06 416.03 442.06 447.19 16.62 6.02 5.38
painting 24.16 8.73 19.76 14.64 8.76 8.73 14.64 15.21 101.45 44.71 39.97
rendering 1280.36 1165.59 1147.44 1146.56 1153.83 1146.56 1153.83 1178.76 11.35 4.86 4.35
CLOSE_MODAL
scripting 193.57 181.07 643.53 197.99 212.29 181.07 197.99 285.69 161.87 70.13 62.73
painting 12.5 4.6 4.52 12.09 9.74 4.52 9.74 8.69 91.83 44.99 40.28
rendering 909.41 880.14 885.53 883.69 871.41 871.41 883.69 886.04 4.29 1.60 1.43
SELECT_WIDGET_MENU_OPEN
scripting 1046.11 926.38 916.61 886.24 925.94 886.24 925.94 940.26 17.00 6.53 5.84
painting 7.97 10.95 5.79 11.28 6.37 5.79 7.97 8.47 64.82 29.99 26.92
rendering 772.98 647.13 644.31 609.78 639.74 609.78 644.31 662.79 24.62 9.56 8.55
SELECT_WIDGET_SELECT_OPTION
scripting 167.83 153.19 153.98 156.06 155.65 153.19 155.65 157.34 9.30 3.80 3.40
painting 9.9 2.02 3.63 2.36 7.94 2.02 3.63 5.17 152.42 68.47 61.32
rendering 323.33 301.1 301.17 300.22 294.66 294.66 301.1 304.1 9.43 3.65 3.26

@shadabbuchh
Copy link

Retested the above issues. Below is the status:

  1. Fixed
  2. Fixed
  3. Will raise a separate issue
  4. Fixed
  5. Will raise a separate issue
  6. Still exists
  7. Still exists
  8. Still exists.

Additionally now text inside some of the drop-downs is getting chipped.
Screenshot 2022-09-23 at 11.00.34 AM.png
Screenshot 2022-09-23 at 11.00.41 AM.png

@albinAppsmith
Copy link
Collaborator Author

/ok-to-test sha=b7daa7c

@shadabbuchh
Copy link

The issue where text of selected options was getting chipped is fixed in this PR.

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3112202683.
Workflow: Appsmith External Integration Test Workflow.
Commit: b7daa7c.
PR: 16718.

@github-actions
Copy link

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

Click to view performance test results

Run 1 (ms) Run 2 (ms) Run 3 (ms) Run 4 (ms) Run 5 (ms) Minimum (ms) Median (ms) Mean (ms) Range (%) SD.Sample (%) SD.Population (%)
SELECT_CATEGORY
scripting 354.37 332.1 352.85 320.35 328.94 320.35 332.1 337.72 10.07 4.48 4.01
painting 3.41 3.04 6.28 4.26 6.09 3.04 4.26 4.62 70.13 32.47 29.00
rendering 103.43 104.61 100.65 103.58 102.77 100.65 103.43 103.01 3.84 1.43 1.28
BIND_TABLE_DATA
scripting 1085.25 1117.21 1080.58 1025.8 1105.22 1025.8 1085.25 1082.81 8.44 3.25 2.90
painting 16.53 16.18 12.63 23.75 17.39 12.63 16.53 17.3 64.28 23.35 20.87
rendering 776.46 801.26 790.82 823.52 787.94 776.46 790.82 796 5.91 2.23 1.99
CLICK_ON_TABLE_ROW
scripting 803.22 857.41 1174.1 868.43 924.46 803.22 868.43 925.52 40.07 15.72 14.06
painting 8.64 10.54 8.62 14.54 10.61 8.62 10.54 10.59 55.90 22.76 20.40
rendering 283.73 285.74 295.03 298.92 292.79 283.73 292.79 291.24 5.22 2.19 1.96
UPDATE_POST_TITLE
scripting 1905.32 1361.7 1350.73 1308.51 1423.76 1308.51 1361.7 1470 40.60 16.79 15.02
painting 12.8 13.74 12.89 14.41 12.99 12.8 12.99 13.37 12.04 5.16 4.64
rendering 583.51 582.57 583.7 582.56 583.64 582.56 583.51 583.2 0.20 0.10 0.09
OPEN_MODAL
scripting 430.74 425.16 475.21 459.52 434.54 425.16 434.54 445.03 11.25 4.81 4.30
painting 12.48 12.63 7.17 11.81 15.75 7.17 12.48 11.97 71.68 25.73 23.06
rendering 1156.05 1167.42 1159.45 1258.5 1141.33 1141.33 1159.45 1176.55 9.96 3.98 3.56
CLOSE_MODAL
scripting 200.27 231.87 205.25 187.62 179.46 179.46 200.27 200.89 26.09 10.00 8.95
painting 9.89 4.08 7.16 4.55 11.19 4.08 7.16 7.37 96.47 42.74 38.26
rendering 934.51 882.48 877.38 883.21 863.28 863.28 882.48 888.17 8.02 3.05 2.73
SELECT_WIDGET_MENU_OPEN
scripting 939.08 929.39 909.08 970.22 920.51 909.08 929.39 933.66 6.55 2.49 2.23
painting 6.13 5.73 11.76 6.9 10.23 5.73 6.9 8.15 73.99 33.01 29.45
rendering 638.19 621.6 625.73 660.78 622.4 621.6 625.73 633.74 6.18 2.61 2.33
SELECT_WIDGET_SELECT_OPTION
scripting 166.5 160.47 162.5 182.35 157.83 157.83 162.5 165.93 14.78 5.85 5.23
painting 7.46 3.79 11.31 2.39 10.4 2.39 7.46 7.07 126.17 55.59 49.79
rendering 311.46 305.05 306.77 314.91 319.35 305.05 311.46 311.51 4.59 1.88 1.68

@albinAppsmith
Copy link
Collaborator Author

/ok-to-test sha=0edb6c7

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3138284923.
Workflow: Appsmith External Integration Test Workflow.
Commit: 0edb6c7.
PR: 16718.

@albinAppsmith
Copy link
Collaborator Author

/ok-to-test sha=4f4ec30

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3138557171.
Workflow: Appsmith External Integration Test Workflow.
Commit: 4f4ec30.
PR: 16718.

@albinAppsmith
Copy link
Collaborator Author

/ok-to-test sha=d06d063

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3138568919.
Workflow: Appsmith External Integration Test Workflow.
Commit: d06d063.
PR: 16718.

@github-actions
Copy link

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

Click to view performance test results

Run 1 (ms) Run 2 (ms) Run 3 (ms) Run 4 (ms) Run 5 (ms) Minimum (ms) Median (ms) Mean (ms) Range (%) SD.Sample (%) SD.Population (%)
SELECT_WIDGET_MENU_OPEN
scripting 973.44 962.81 947.89 994.66 931.75 931.75 962.81 962.11 6.54 2.50 2.23
painting 4.66 7.56 5.34 5.45 9.7 4.66 5.45 6.54 77.06 31.65 28.29
rendering 635.36 660.05 660.95 657 639.25 635.36 657 650.52 3.93 1.88 1.68
SELECT_WIDGET_SELECT_OPTION
scripting 157.2 155.4 186.74 160.51 150.67 150.67 157.2 162.1 22.25 8.77 7.85
painting 4.16 3.47 5.29 9.04 4.96 3.47 4.96 5.38 103.53 40.15 35.87
rendering 311.73 309.2 340.06 301.44 294.32 294.32 309.2 311.35 14.69 5.60 5.01
SELECT_CATEGORY
scripting 1100.56 373.4 375.75 360.3 360.3 374.575 552.5 133.98 66.14 57.28
painting 3.02 4.68 3.73 12.06 3.02 4.205 5.87 154.00 71.21 61.67
rendering 102.49 105.19 109.78 104.45 102.49 104.82 105.48 6.91 2.93 2.53
BIND_TABLE_DATA
scripting 972.89 1094.75 1047.43 1117.94 972.89 1071.0900000000001 1058.25 13.71 6.05 5.24
painting 11.44 20.31 22.71 17.01 11.44 18.66 17.87 63.07 27.31 23.67
rendering 774.98 828.04 869.71 805.54 774.98 816.79 819.57 11.56 4.87 4.21
CLICK_ON_TABLE_ROW
scripting 882.93 859.23 872.12 901.47 859.23 877.525 878.94 4.81 2.03 1.76
painting 10.91 16.21 17.88 9.7 9.7 13.56 13.68 59.80 29.09 25.22
rendering 313.46 291.77 296.88 300.8 291.77 298.84000000000003 300.73 7.21 3.08 2.67
UPDATE_POST_TITLE
scripting 1399.06 1361.5 1916.94 1404.97 1361.5 1402.0149999999999 1520.62 36.53 17.42 15.09
painting 12.99 15.51 13.96 13.14 12.99 13.55 13.9 18.13 8.27 7.19
rendering 590.16 609.44 620.65 598.37 590.16 603.905 604.65 5.04 2.19 1.90
OPEN_MODAL
scripting 468.75 510.7 469.97 492.08 468.75 481.025 485.38 8.64 4.12 3.57
painting 20.78 17.15 11.17 13.76 11.17 15.454999999999998 15.71 61.17 26.54 22.98
rendering 1187.9 1191.47 1240.03 1231.7 1187.9 1211.585 1212.77 4.30 2.22 1.92
CLOSE_MODAL
scripting 188.52 246.44 200.4 184.85 184.85 194.46 205.05 30.04 13.84 11.99
painting 5.13 22.25 10.31 5.32 5.13 7.815 10.75 159.26 74.70 64.74
rendering 891.9 937.63 913.01 895.08 891.9 904.0450000000001 909.4 5.03 2.31 2.00

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Design System Pod Appsmith design system related issues Medium Issues that frustrate users due to poor UX Production
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Google Sheets | Drop-downs close abruptly when a space or wrong/random text is entered
4 participants