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

chore: table widget cell tooltip should be calculated only on hover #19056

Merged
merged 5 commits into from
Jan 4, 2023
Merged

chore: table widget cell tooltip should be calculated only on hover #19056

merged 5 commits into from
Jan 4, 2023

Conversation

sbalaji1192
Copy link
Contributor

@sbalaji1192 sbalaji1192 commented Dec 20, 2022 •
edited
Loading

Description

Prior to this PR, we were calculating whether a cell has overflowing content or not when the table gets loaded, to show a tooltip on hover. But this is affecting the performance of the table, now we're calculating for a cell only when the user hovers the cell.

Fixes #18995

Type of change

  • chore

How Has This Been Tested?

  • Manual
  • Cypress

Test Plan

Add Testsmith test cases links that relate to this PR

Issues raised during DP testing

Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR)

Checklist:

Dev activity

  • 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
  • PR is being merged under a feature flag

QA activity:

  • Test plan has been approved by relevant developers
  • Test plan has been peer reviewed by QA
  • Cypress test cases have been added and approved by either SDET or manual QA
  • Organized project review call with relevant stakeholders after Round 1/2 of QA
  • Added Test Plan Approved label after reveiwing all Cypress test

@vercel
Copy link

vercel bot commented Dec 20, 2022 •
edited
Loading

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

Name Status Preview Updated
appsmith ✅ Ready (Inspect) Visit Preview Jan 4, 2023 at 0:32AM (UTC)

@github-actions github-actions bot added Widgets Product This label groups issues related to widgets Performance Pod All things related to Appsmith performance Table Widget V2 Issues related to Table Widget V2 Task A simple Todo skip-changelog Adding this label to a PR prevents it from being listed in the changelog labels Dec 20, 2022
@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=ca987b9

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3738473761.
Workflow: Appsmith External Integration Test Workflow.
Commit: ca987b9.
PR: 19056.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=19056&runId=3738473761_1

@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=a8ce0a2

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3746506910.
Workflow: Appsmith External Integration Test Workflow.
Commit: a8ce0a2.
PR: 19056.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=19056&runId=3746506910_1

@github-actions
Copy link

The following are new failures, please fix them before merging the PR cypress/integration/Smoke_TestSuite/ClientSideTests/Onboarding/GuidedTour_spec.js

@github-actions
Copy link

The following are new failures, please fix them before merging the PR cypress/integration/Smoke_TestSuite/ClientSideTests/Onboarding/GuidedTour_spec.js

@Aishwarya-U-R
Copy link
Contributor

/ok-to-test sha=486728e

@github-actions
Copy link

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3749282000.
Workflow: Appsmith External Integration Test Workflow.
Commit: 486728e.
PR: 19056.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=19056&runId=3749282000_1

@kamakshibhat-appsmith
Copy link

Observations :

  1. Tooltips are not getting dissmissed in the first run
    https://www.loom.com/share/3f2d7a7116394b2e80ef62bb66b73343

@github-actions
Copy link

The following are new failures, please fix them before merging the PR cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Form/FormWidget_spec.js

@github-actions
Copy link

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 Dec 29, 2022
@sbalaji1192 sbalaji1192 force-pushed the chore/table-celltooltip-refactor branch from 486728e to a51e542 Compare January 2, 2023 06:02
@laveena-en
Copy link
Contributor

@sbalaji1192 looks like tooltips aren’t being dismissed intermittently. Can you please have a look?

https://ivP6ez0lJB.vmaker.com/record/XF7sj0CnNJGoCdxz

@laveena-en
Copy link
Contributor

Re-tested and LGTM.

@github-actions github-actions bot removed the Stale label Jan 2, 2023
@sbalaji1192
Copy link
Contributor Author

/ok-to-test sha=6a5730e

@github-actions
Copy link

github-actions bot commented Jan 3, 2023

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3826954916.
Workflow: Appsmith External Integration Test Workflow.
Commit: 6a5730e.
PR: 19056.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=19056&runId=3826954916_1

@github-actions
Copy link

github-actions bot commented Jan 3, 2023

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3827451436.
Workflow: Appsmith External Integration Test Workflow.
Commit: 6a5730e.
PR: 19056.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=19056&runId=3827451436_1

@github-actions
Copy link

github-actions bot commented Jan 3, 2023

The following are new failures, please fix them before merging the PR cypress/integration/Smoke_TestSuite/Application/CommunityIssues_Spec.ts
cypress/integration/Smoke_TestSuite/Application/EchoApiCMS_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Autocomplete_JS_spec.ts
cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug9334_Spec.ts
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitDiscardChange/DiscardChanges_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitImport/GitImport_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/DeleteBranch_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/GitBugs_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/Git_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/SwitchBranches_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/IDE/Command_Click_Navigation_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Onboarding/GuidedTour_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/VisualTests/AppPageLayout_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/VisualTests/JSEditorIndent_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/VisualTests/WidgetsLayout_spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/Datasources/SMTPDatasource_spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/Datatypes/MySQL_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/Datatypes/MySQL_false_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/Mongo_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/MySQL1_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/MySQL2_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/Postgres1_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/Postgres2_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/Postgres_DataTypes/Character_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/EmptyDataSource_spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/Mongo_Spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/Mongo_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/Postgres_Spec.js

@github-actions
Copy link

github-actions bot commented Jan 3, 2023

The following are new failures, please fix them before merging the PR cypress/integration/Smoke_TestSuite/Application/CommunityIssues_Spec.ts
cypress/integration/Smoke_TestSuite/Application/EchoApiCMS_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug9334_Spec.ts
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitDiscardChange/DiscardChanges_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitImport/GitImport_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/DeleteBranch_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/GitBugs_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/Git_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/SwitchBranches_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/IDE/Command_Click_Navigation_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Templates/Fork_Template_To_App_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/VisualTests/DatasourcePageLayout_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/VisualTests/JSEditorIndent_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/VisualTests/WidgetsLayout_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Widgets/Form/FormWidget_spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/Datasources/SMTPDatasource_spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/Datatypes/MySQL_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/Datatypes/MySQL_false_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/Mongo_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/MySQL1_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/MySQL2_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/Postgres1_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/Postgres2_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/Postgres_DataTypes/Character_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/Mongo_Spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/Mongo_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/Postgres_Spec.js

@github-actions
Copy link

github-actions bot commented Jan 3, 2023

The following are new failures, please fix them before merging the PR cypress/integration/Smoke_TestSuite/Application/CommunityIssues_Spec.ts
cypress/integration/Smoke_TestSuite/Application/EchoApiCMS_spec.js
cypress/integration/Smoke_TestSuite/Application/EchoApiCMS_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Autocomplete_JS_spec.ts
cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Bug9334_Spec.ts
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitDiscardChange/DiscardChanges_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitImport/GitImport_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/DeleteBranch_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/GitBugs_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/GitSyncedApps_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/Git_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Git/GitSync/SwitchBranches_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/IDE/Command_Click_Navigation_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/Onboarding/GuidedTour_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/VisualTests/AppPageLayout_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/VisualTests/JSEditorIndent_spec.js
cypress/integration/Smoke_TestSuite/ClientSideTests/VisualTests/WidgetsLayout_spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/Datasources/SMTPDatasource_spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/Datatypes/MySQL_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/Datatypes/MySQL_false_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/Mongo_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/MySQL1_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/MySQL2_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/Postgres1_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/GenerateCRUD/Postgres2_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/Postgres_DataTypes/Character_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/EmptyDataSource_spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/Mongo_Spec.js
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/Mongo_Spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/Postgres_Spec.js

@Aishwarya-U-R
Copy link
Contributor

/ok-to-test sha=f1cf854

@github-actions
Copy link

github-actions bot commented Jan 4, 2023

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3834081287.
Workflow: Appsmith External Integration Test Workflow.
Commit: f1cf854.
PR: 19056.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-638dd7cd2913ba43778b915e?pr=19056&runId=3834081287_1

@github-actions
Copy link

github-actions bot commented Jan 4, 2023

The following are new failures, please fix them before merging the PR cypress/integration/Smoke_TestSuite/ClientSideTests/BugTests/Autocomplete_JS_spec.ts
cypress/integration/Smoke_TestSuite/ServerSideTests/QueryPane/EmptyDataSource_spec.js

@sbalaji1192 sbalaji1192 merged commit db399c9 into release Jan 4, 2023
@sbalaji1192 sbalaji1192 deleted the chore/table-celltooltip-refactor branch January 4, 2023 04:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ashit-rath ashit-rath ashit-rath approved these changes

Assignees
No one assigned
Labels
Performance Pod All things related to Appsmith performance skip-changelog Adding this label to a PR prevents it from being listed in the changelog Table Widget V2 Issues related to Table Widget V2 Task A simple Todo 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.

[Task]: Table widget cell tooltip should be initialised only when a cell is hovered
5 participants
@sbalaji1192 @Aishwarya-U-R @kamakshibhat-appsmith @laveena-en @ashit-rath