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

feat: Select Reskinning #16173

Merged
merged 56 commits into from
Aug 24, 2022
Merged

feat: Select Reskinning #16173

merged 56 commits into from
Aug 24, 2022

Conversation

jsartisan
Copy link
Contributor

@jsartisan jsartisan commented Aug 21, 2022

Description

Current select widgets have many minor inconsistencies. This PR fixes them

Fixes #10988
Fixes #10991
Fixes #10984
Fixes #10990

Type of change

  • Bug fix

How Has This Been Tested?

  • Manually

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

@github-actions github-actions bot added the Enhancement New feature or request label Aug 21, 2022
@github-actions
Copy link

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

@github-actions github-actions bot added Widgets Product This label groups issues related to widgets Low An issue that is neither critical nor breaks a user flow MultiSelect Widget Issues related to MultiSelect Widget MultiTree Select Widget Issues related to MultiTree Select Widget Select Widget Select or dropdown widget TreeSelect Issues related to TreeSelect Widget labels Aug 23, 2022
@github-actions
Copy link

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

@laveena-en
Copy link
Contributor

laveena-en commented Aug 24, 2022

@jsartisan For select widget:

  1. Can we add some kind of hover state or emphasis to depict that the user has hovered over an option for the selected and first item in the dropdown as well to make it seem more responsive?

image

  1. Error state should also be seen when we hover over the widget when it's set to required

image

@laveena-en
Copy link
Contributor

laveena-en commented Aug 24, 2022

@jsartisan Also, the error state is not being seen on the Tree-select widget when it is set to required.
https://www.loom.com/share/7de40bd8f7974421a3afbfeb261d79fa

UPDATE: A separate issue has been raised for this #16264

@jsartisan
Copy link
Contributor Author

/ok-to-test sha=9328f97

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2918908190.
Workflow: Appsmith External Integration Test Workflow.
Commit: 9328f97.
PR: 16173.

Copy link
Contributor

@keyurparalkar keyurparalkar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2918908190.
Commit: 9328f97.
Results:

Click to view performance test results

Run 1 (ms) Run 2 (ms) Run 3 (ms) Run 4 (ms) Run 5 (ms) Median (ms) Mean (ms) Range (%) SD.Sample (%) SD.Population (%)
SELECT_CATEGORY
scripting 277.55 294.63 294.36 1001.37 287.7 294.36 431.12 167.89 73.96 66.15
painting 3.26 2.93 3.19 4.07 4.71 3.26 3.63 49.04 20.39 18.18
rendering 106.39 103.59 107.16 107.2 108.71 107.16 106.61 4.80 1.77 1.59
BIND_TABLE_DATA
scripting 1459.66 1476.44 1479.81 1467.15 1451.05 1467.15 1466.82 1.96 0.81 0.72
painting 15.65 12.84 11.85 12.64 12.57 12.64 13.11 28.99 11.21 9.99
rendering 420.21 406.97 413.19 416.77 411.21 413.19 413.67 3.20 1.23 1.10
CLICK_ON_TABLE_ROW
scripting 712.52 819.49 847.63 875.47 828.8 828.8 816.78 19.95 7.60 6.80
painting 10.37 9.27 14.22 9.02 8.02 9.27 10.18 60.90 23.67 21.12
rendering 293.24 287.71 308.01 306.75 320.51 306.75 303.24 10.82 4.28 3.83
UPDATE_POST_TITLE
scripting 1379.57 1295.55 1228.09 1292.79 1336.89 1295.55 1306.58 11.59 4.32 3.86
painting 13.68 12.31 12.66 12.68 20.29 12.68 14.32 55.73 23.53 21.09
rendering 446.25 457.62 436.63 439.44 452.27 446.25 446.44 4.70 1.95 1.75
OPEN_MODAL
scripting 476.05 548.48 475.51 507.17 497.48 497.48 500.94 14.57 5.97 5.34
painting 13.3 8.2 7.12 13.06 10.73 10.73 10.48 58.97 26.62 23.85
rendering 373.49 375.54 374.91 374.14 373.23 374.14 374.26 0.62 0.26 0.23
CLOSE_MODAL
scripting 292.3 300.88 294.39 778.46 296.44 296.44 392.49 123.87 54.98 49.17
painting 5.49 4.76 11.27 7.03 4.29 5.49 6.57 106.24 43.07 38.51
rendering 605.41 608.02 614.49 623.81 616.39 614.49 613.62 3.00 1.18 1.06
SELECT_WIDGET_MENU_OPEN
scripting 1098.65 1066.07 1100.01 1051.69 1095.19 1095.19 1082.32 4.46 2.04 1.82
painting 7.07 5.87 10.78 4.33 6.41 6.41 6.89 93.61 34.83 31.06
rendering 610.94 604.58 602.74 614.56 613.24 610.94 609.21 1.94 0.87 0.77
SELECT_WIDGET_SELECT_OPTION
scripting 138.7 146.85 147.11 140.5 145.64 145.64 143.76 5.85 2.71 2.42
painting 8.93 2.02 2.18 9.4 4.31 4.31 5.37 137.43 66.85 59.78
rendering 301.24 310.88 301.62 320.65 314.11 310.88 309.7 6.27 2.69 2.41

@jsartisan jsartisan merged commit 8884345 into release Aug 24, 2022
@jsartisan jsartisan deleted the reskinning/select branch August 24, 2022 15:01
@laveena-en
Copy link
Contributor

@jsartisan Noticing some weird behaviour with select widget on release.
The widget looks disabled even when the option is turned off. Can you please check?
https://www.loom.com/share/d272006d6c1445f399ea6fa8a44f3040

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request Low An issue that is neither critical nor breaks a user flow MultiSelect Widget Issues related to MultiSelect Widget MultiTree Select Widget Issues related to MultiTree Select Widget Select Widget Select or dropdown widget TreeSelect Issues related to TreeSelect Widget Widgets Product This label groups issues related to widgets
Projects
None yet
4 participants