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: theme property pane reset on hover over the widget #13716

Merged
merged 5 commits into from
May 13, 2022
Merged

fix: theme property pane reset on hover over the widget #13716

merged 5 commits into from
May 13, 2022

Conversation

keyurparalkar
Copy link
Contributor

@keyurparalkar keyurparalkar commented May 10, 2022 •
edited by github-actions bot
Loading

Description

Summary of changes

  • Added SELECT_WIDGET_INIT and START_CANVAS_SELECTION actions in the appThemingReducer.
  • Set the appStack to [].

Motivation for the above changes

  • The issue was occurring due to appStack getting set to [] on FOCUS_WIDGET action
  • The expected behaviour is the appStack should be blank whenever the user clicks on a widget or on the canvas.

Fixes #13639

Type of change

Please delete options that are not relevant.

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

How Has This Been Tested?

Manually tested

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 increased 
// Code coverage diff between base branch:release and head branch: fix/fix-theme-pane-reset-on-hover
Status File % Stmts % Branch % Funcs % Lines
🟢 total 56.61 (0) 38.55 (0.01) 35.9 (0.01) 56.85 (0)
🟢 app/client/src/reducers/uiReducers/appThemingReducer.ts 29.17 (3.08) 100 (0) 13.33 (6.19) 29.17 (3.08)
🟢 app/client/src/selectors/propertyPaneSelectors.tsx 88.89 (2.78) 70.37 (5.56) 91.67 (0) 88.71 (0)

@keyurparalkar keyurparalkar self-assigned this May 10, 2022
@vercel
Copy link

vercel bot commented May 10, 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 13, 2022 at 8:32AM (UTC)

@github-actions github-actions bot added App Theming Product Items that are related to the App level theming controls epic Widgets Product This label groups issues related to widgets Bug Something isn't working Medium Issues that frustrate users due to poor UX Release UI Building Pod labels May 10, 2022
@github-actions
Copy link

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

@keyurparalkar
Copy link
Contributor Author

/ok-to-test sha=9270165

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2300246405.
Workflow: Appsmith External Integration Test Workflow.
Commit: 9270165.
PR: 13716.

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2300246405.
Commit: 9270165.
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 860.35 747.08 865.38 768.64 825.35 825.35 813.36 6.58 5.88
painting 4.58 3.98 4.13 6.62 6.71 4.58 5.2 25.96 23.27
rendering 323.13 298.83 324.68 321.42 343.03 323.13 322.22 4.88 4.36
BIND_TABLE_DATA
scripting 2201.96 2304.62 2324.71 2277.15 2361.89 2304.62 2294.07 2.62 2.34
painting 18.24 12.96 23.96 23.28 8.9 18.24 17.47 37.32 33.43
rendering 657.06 678.61 666.11 730.75 662.54 666.11 679.01 4.42 3.95
CLICK_ON_TABLE_ROW
scripting 2076.25 2417.83 1992.84 2350.35 2150.65 2150.65 2197.58 8.23 7.36
painting 21.85 21.25 21.99 27.45 16.26 21.85 21.76 18.24 16.31
rendering 343.25 335.19 334.25 350.52 326.78 335.19 338 2.70 2.41
UPDATE_POST_TITLE
scripting 3401.3 3264.75 3429.08 3408.79 4078.61 3408.79 3516.51 9.12 8.16
painting 18.51 29.78 29.78 34.78 24.87 29.78 27.54 22.33 19.97
rendering 446.06 399.72 452.11 467.46 437.63 446.06 440.6 5.75 5.14
OPEN_MODAL
scripting 2881.14 1376.63 1476.09 1612.47 1440.87 1476.09 1757.44 36.08 32.27
painting 12.47 13.21 14.8 37.61 12.65 13.21 18.15 60.17 53.83
rendering 439.5 451.53 461.56 533.83 504.52 461.56 478.19 8.28 7.41
CLOSE_MODAL
scripting 2832.63 1036.74 1680.63 766.16 815.68 1036.74 1426.37 60.74 54.33
painting 5.84 6.14 4.64 5.44 7.31 5.84 5.87 16.70 14.99
rendering 381.58 401.57 348.86 372.38 442.42 381.58 389.36 9.04 8.09
SELECT_WIDGET_MENU_OPEN
scripting 1560.24 1718.57 1516.54 1453.27 1478.34 1516.54 1545.39 6.79 6.07
painting 11.25 7.5 11.69 7.07 9.54 9.54 9.41 22.42 19.98
rendering 513.21 624.78 509.46 512.55 534.92 513.21 538.98 9.09 8.14
SELECT_WIDGET_SELECT_OPTION
scripting 271.47 293.73 266.5 262.1 499.23 271.47 318.61 31.92 28.55
painting 3.12 5.43 5.87 4.3 3.54 4.3 4.45 26.52 23.82
rendering 17.38 18.51 18.34 16.89 22.48 18.34 18.72 11.81 10.52

@github-actions
Copy link

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

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Median Mean SD.Sample SD.Population
SELECT_WIDGET_MENU_OPEN
scripting 1967.38 2028 2043.87 2099.51 1948.4 2028 2017.43 3.02 2.70
painting 15.23 14.7 12.79 15.48 12.76 14.7 14.19 9.30 8.32
rendering 687.59 751.18 705.97 701.87 704.13 704.13 710.15 3.39 3.03
SELECT_WIDGET_SELECT_OPTION
scripting 289.36 303.63 338.88 366.62 338.75 338.75 327.45 9.43 8.43
painting 8.62 4.43 5.37 4.41 9.61 5.37 6.49 37.75 33.90
rendering 26.37 20.77 21.26 21.47 20.3 21.26 22.03 11.21 10.03

@vivekverma2312
Copy link

@keyurparalkar Tested and the issue is fixed.

@keyurparalkar
Copy link
Contributor Author

/ok-to-test sha=8c14b0e

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2306383066.
Workflow: Appsmith External Integration Test Workflow.
Commit: 8c14b0e.
PR: 13716.

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2306383066.
Commit: 8c14b0e.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Median Mean SD.Sample SD.Population
SELECT_WIDGET_MENU_OPEN
scripting 1614.89 1450.83 1399.98 1575.98 1693.83 1575.98 1547.1 7.77 6.95
painting 5.15 11.23 7.66 9.13 7.85 7.85 8.2 27.07 24.27
rendering 530.06 488.74 486.7 535.11 529.37 529.37 514 4.69 4.19
SELECT_WIDGET_SELECT_OPTION
scripting 235.69 310.21 247.17 247.99 227.65 247.17 253.74 12.88 11.52
painting 3.6 6.19 5.47 3.11 3.36 3.6 4.35 31.95 28.51
rendering 16.5 16.96 15.1 17.35 16.01 16.5 16.38 5.37 4.76

@keyurparalkar
Copy link
Contributor Author

/ok-to-test sha=d8c26bc

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2310997499.
Workflow: Appsmith External Integration Test Workflow.
Commit: d8c26bc.
PR: 13716.

@keyurparalkar
Copy link
Contributor Author

/ok-to-test sha=0bcd21f

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2318323908.
Workflow: Appsmith External Integration Test Workflow.
Commit: 0bcd21f.
PR: 13716.

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2318323908.
Commit: 0bcd21f.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Median Mean SD.Sample SD.Population
SELECT_WIDGET_MENU_OPEN
scripting 2403.83 2406.31 2355.09 2424 2402.47 2403.83 2398.34 1.07 0.96
painting 12.42 15.76 16.26 26.57 11.5 15.76 16.5 36.30 32.48
rendering 837.41 853.12 816.58 836.8 824.92 836.8 833.77 1.66 1.49
SELECT_WIDGET_SELECT_OPTION
scripting 374.18 380.89 405.44 377.75 395.41 380.89 386.73 3.42 3.05
painting 19.05 13.95 5.92 5.47 5.59 5.92 10 62.10 55.50
rendering 23 22.22 22.55 24.72 21.96 22.55 22.89 4.76 4.28

1 similar comment
@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2318323908.
Commit: 0bcd21f.
Results:

Click to view performance test results

Run 1 Run 2 Run 3 Run 4 Run 5 Median Mean SD.Sample SD.Population
SELECT_WIDGET_MENU_OPEN
scripting 2403.83 2406.31 2355.09 2424 2402.47 2403.83 2398.34 1.07 0.96
painting 12.42 15.76 16.26 26.57 11.5 15.76 16.5 36.30 32.48
rendering 837.41 853.12 816.58 836.8 824.92 836.8 833.77 1.66 1.49
SELECT_WIDGET_SELECT_OPTION
scripting 374.18 380.89 405.44 377.75 395.41 380.89 386.73 3.42 3.05
painting 19.05 13.95 5.92 5.47 5.59 5.92 10 62.10 55.50
rendering 23 22.22 22.55 24.72 21.96 22.55 22.89 4.76 4.28

@keyurparalkar keyurparalkar merged commit a5f426d into release May 13, 2022
@keyurparalkar keyurparalkar deleted the fix/fix-theme-pane-reset-on-hover branch May 13, 2022 10:40
keyurparalkar added a commit that referenced this pull request May 13, 2022
@keyurparalkar
Revert "fix: theme property pane reset on hover over the widget (#13716…
2271f64 
…)"

This reverts commit a5f426d.
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

@keyurparalkar keyurparalkar

Labels
App Theming Product Items that are related to the App level theming controls epic Bug Something isn't working Medium Issues that frustrate users due to poor UX Release Widgets Product This label groups issues related to widgets
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: App theming pane resets when hovering over a widget
3 participants
@keyurparalkar @vivekverma2312 @jsartisan