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: Deprecation warning #13381

Merged
merged 14 commits into from
May 25, 2022
Merged

feat: Deprecation warning #13381

merged 14 commits into from
May 25, 2022

Conversation

rahulramesha
Copy link
Contributor

@rahulramesha rahulramesha commented Apr 27, 2022

Description

This PR, helps in Adding Deprecation message to all the Deprected widgets and also adds code to enable showing the message to the Deprecation messages in the future as well, just by adding isDeprecated: true to the widget config

Fixes #12291

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Manual UI and Cypress Test

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: deprecationWarning 
Status File % Stmts % Branch % Funcs % Lines
🟒 total 56.63 (0.01) 38.65 (0.02) 35.96 (0.02) 56.88 (0.01)
🟒 app/client/src/ce/constants/messages.ts 77.92 (0.14) 100 (0) 34.67 (0.41) 81.94 (0.2)
✨ πŸ†• app/client/src/components/ads/BannerMessage.tsx 75 83.33 0 75
🟒 app/client/src/pages/Editor/utils.ts 25.6 (1.83) 16.85 (2.73) 39.13 (2.77) 23.68 (2.06)
🟒 app/client/src/pages/Editor/PropertyPane/PropertyPaneView.tsx 89.74 (3.07) 69.57 (-4.11) 60 (0) 94.59 (1.73)
πŸ”΄ 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 27, 2022

The latest updates on your projects. Learn more about Vercel for Git β†—οΈŽ

Name Status Preview Updated
appsmith βœ… Ready (Inspect) Visit Preview May 23, 2022 at 4:40PM (UTC)

@github-actions github-actions bot added All Widgets Issues related to all widgets App Viewers Pod This label assigns issues to the app viewers pod Enhancement New feature or request Needs Design needs design or changes to design UI Builders Pod Issues that UI Builders face using appsmith UI Building Pod labels Apr 27, 2022
@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.

5 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
Copy link

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

@@ -0,0 +1,46 @@
import React from "react";
Copy link
Contributor

Choose a reason for hiding this comment

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

@rahulramesha This seems like something which belongs in the design system. Could you check with the design team?
I am asking because, we need to avoid using ad-hoc components like these, which can effect UI consistency in the long run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, we will need a generic Banner component that takes in inputs for Icons, colors, text and sizes. Do I rewrite the Banner as a generic component here or take that up as a separate issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a new Component with sufficient customisation.

@rahulramesha rahulramesha dismissed stale reviews from riodeuno and aswathkk via 8416300 May 17, 2022 14:46
@rahulramesha
Copy link
Contributor Author

Updated the message to,
image

@rahulramesha
Copy link
Contributor Author

/ok-to-test sha=8416300

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2339495187.
Workflow: Appsmith External Integration Test Workflow.
Commit: 8416300.
PR: 13381.

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2339495187.
Commit: 8416300.
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 2036.37 1985.43 1998.46 1986.36 1974.58 1986.36 1996.24 1.20 1.07
painting 14.75 11.28 12.01 11.32 11.99 11.99 12.27 11.65 10.43
rendering 627.85 640.01 632.1 637.51 638.01 637.51 635.1 0.79 0.70
SELECT_WIDGET_SELECT_OPTION
scripting 360.39 420.01 376.56 353.29 370.2 370.2 376.09 6.95 6.21
painting 9.39 8.6 3.83 4.69 4.73 4.73 6.25 40.80 36.48
rendering 18.38 25.84 17.46 16.74 17.59 17.59 19.2 19.58 17.50

1 similar comment
@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2339495187.
Commit: 8416300.
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 2036.37 1985.43 1998.46 1986.36 1974.58 1986.36 1996.24 1.20 1.07
painting 14.75 11.28 12.01 11.32 11.99 11.99 12.27 11.65 10.43
rendering 627.85 640.01 632.1 637.51 638.01 637.51 635.1 0.79 0.70
SELECT_WIDGET_SELECT_OPTION
scripting 360.39 420.01 376.56 353.29 370.2 370.2 376.09 6.95 6.21
painting 9.39 8.6 3.83 4.69 4.73 4.73 6.25 40.80 36.48
rendering 18.38 25.84 17.46 16.74 17.59 17.59 19.2 19.58 17.50

riodeuno
riodeuno previously approved these changes May 23, 2022
@@ -122,7 +131,23 @@ function PropertyPaneView(
/>
)}
<PropertyPaneConnections widgetName={widgetProperties.widgetName} />

{isWidgetDeprecated(widgetProperties.type) && (
<BannerMessage
Copy link
Contributor

Choose a reason for hiding this comment

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

This component is a little hard to read, can we create separate variables, which we can use for props? I am referring especially to createMessage.

@rahulramesha
Copy link
Contributor Author

/ok-to-test sha=c817207

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2372126426.
Workflow: Appsmith External Integration Test Workflow.
Commit: c817207.
PR: 13381.

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2372126426.
Commit: c817207.
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 732.2 1176.79 794.2 1077.99 2175.85 1077.99 1191.41 48.78 43.63
painting 4.52 10.05 6.58 7.69 11.17 7.69 8 33.38 29.88
rendering 295.99 510.92 359.92 511.58 409.33 409.33 417.55 22.63 20.24
BIND_TABLE_DATA
scripting 2235.92 2411.41 2486.65 2702.72 2467.87 2467.87 2460.91 6.81 6.09
painting 26.13 17.93 38.45 27.17 30.88 27.17 28.11 26.57 23.76
rendering 807.32 794.31 957.78 1010.63 780.48 807.32 870.1 12.21 10.92
CLICK_ON_TABLE_ROW
scripting 2516.5 3633.84 3338.19 3463.77 2860.36 3338.19 3162.53 14.60 13.06
painting 52.11 66.96 54.79 46.59 35.06 52.11 51.1 22.82 20.41
rendering 443.29 1002.92 772.71 819.87 433.8 772.71 694.52 35.85 32.07
UPDATE_POST_TITLE
scripting 3229.13 3789.06 4269.85 3739.47 4080.98 3789.06 3821.7 10.36 9.27
painting 18.7 27.24 26.06 29.54 36.45 27.24 27.6 23.19 20.72
rendering 467.78 573.45 606.84 553.04 547.28 553.04 549.68 9.35 8.36
OPEN_MODAL
scripting 3089.4 1697.17 1729.89 1459.22 1630.22 1697.17 1921.18 34.42 30.79
painting 16.54 21.42 18.22 13.7 14.94 16.54 16.96 17.81 15.92
rendering 548.42 646 587.52 494.77 564.66 564.66 568.27 9.73 8.70
CLOSE_MODAL
scripting 872.47 817.24 1399.68 857.95 877.82 872.47 965.03 25.30 22.63
painting 8.56 10.41 8.71 7.47 10.3 8.71 9.09 13.75 12.32
rendering 471.73 414.88 709.2 492.67 449.5 471.73 507.6 22.92 20.50
SELECT_WIDGET_MENU_OPEN
scripting 1690.96 1857.11 1605.29 1582.46 1817.8 1690.96 1710.72 7.21 6.45
painting 6.61 9.03 6.93 6.46 16.72 6.93 9.15 47.65 42.62
rendering 534.96 675.76 562.16 531.28 662.93 562.16 593.42 11.88 10.62
SELECT_WIDGET_SELECT_OPTION
scripting 250.98 368.51 247.93 222.86 327.46 250.98 283.55 21.71 19.42
painting 6.32 7.67 3.2 2.58 4.08 4.08 4.77 45.07 40.46
rendering 17.76 26.69 16.78 14.46 23.26 17.76 19.79 25.42 22.74

@rahulramesha
Copy link
Contributor Author

/ok-to-test sha=e3bbb66

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/2372856431.
Workflow: Appsmith External Integration Test Workflow.
Commit: e3bbb66.
PR: 13381.

@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2372856431.
Commit: e3bbb66.
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 2471.58 2505.02 2527.12 2740.31 2429.59 2505.02 2534.72 4.76 4.26
painting 14.54 12.06 15.58 19.07 18.09 15.58 15.87 17.71 15.82
rendering 863.32 909.99 855.21 970.75 858.96 863.32 891.65 5.55 4.96
SELECT_WIDGET_SELECT_OPTION
scripting 357.81 421.7 387.74 363.04 417.08 387.74 389.47 7.60 6.80
painting 18.41 7.42 5.51 9.53 5.68 7.42 9.31 57.36 51.34
rendering 31.87 31.06 22.43 23.65 27 27 27.2 15.63 13.97

1 similar comment
@github-actions
Copy link

UI Performance test run logs and artifacts: https://github.com/appsmithorg/appsmith/actions/runs/2372856431.
Commit: e3bbb66.
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 2471.58 2505.02 2527.12 2740.31 2429.59 2505.02 2534.72 4.76 4.26
painting 14.54 12.06 15.58 19.07 18.09 15.58 15.87 17.71 15.82
rendering 863.32 909.99 855.21 970.75 858.96 863.32 891.65 5.55 4.96
SELECT_WIDGET_SELECT_OPTION
scripting 357.81 421.7 387.74 363.04 417.08 387.74 389.47 7.60 6.80
painting 18.41 7.42 5.51 9.53 5.68 7.42 9.31 57.36 51.34
rendering 31.87 31.06 22.43 23.65 27 27 27.2 15.63 13.97

@rahulramesha rahulramesha enabled auto-merge (squash) May 24, 2022 06:52
@rahulramesha rahulramesha merged commit a2688bb into release May 25, 2022
@rahulramesha rahulramesha deleted the deprecationWarning branch May 25, 2022 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
All Widgets Issues related to all widgets App Viewers Pod This label assigns issues to the app viewers pod Enhancement New feature or request Needs Design needs design or changes to design Stale UI Builders Pod Issues that UI Builders face using appsmith
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature]: deprecation warning on widgets
3 participants