Feature/2111 toggle header options by wmdev0808 · Pull Request #4580 · appsmithorg/appsmith

wmdev0808 · 2021-05-19T11:08:10Z

Description

Add the header options section for the visibility on the property pane
Render conditionally the header part based on the setting

Fixes #2111

Type of change

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been 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: feature/2111-toggle-header-options

Status	File	% Stmts	% Branch	% Funcs	% Lines
🟢	total	51.68 (0.02)	32.85 (0.01)	29.59 (0.02)	52.15 (0.01)
🔴	app/client/src/components/designSystems/appsmith/TableComponent/TableStyledWrappers.tsx	35.37 (0)	15.38 (-0.37)	0 (0)	35.37 (0)
🟢	app/client/src/utils/WidgetPropsUtils.tsx	64.2 (0.22)	52.31 (-0.01)	54.17 (0)	63.58 (0.24)
🟢	app/client/src/utils/migrations/TableWidget.ts	78.7 (3.17)	63.46 (2.59)	78.57 (3.57)	81 (2.84)
🔴	app/client/src/widgets/TableWidget/index.tsx	9.31 (-0.15)	0 (0)	10.64 (0)	9.97 (-0.24)

-- Add the header options section for the visibility setting -- Render conditionally the header part based on the setting

-- Remove JS convertible -- Set isBindProperty and isTriggerProperty to false

-- PositionedContainer : Add padding-bottom: 42px in case of Table widget

-- Remove extra space when the header options is all hidden

This reverts commit 1d3c2da.

-- Apply some cleanup

-- Adjust pageSize -- Change column header height -- Restore the logic for renderEmptyRow

somangshu · 2021-05-19T14:31:28Z

The feature works now. But there is an issue with old tables. There are too many extra rows. This doesn't happen for the new table widgets dropped on the canvas. @riodeuno Don't know if some kinda migration is required here.

@vivekverma2312 can we test this Deploy preview thoroughly.

somangshu · 2021-05-19T14:38:08Z

/ok-to-test sha=c1557c1

vivekverma2312 · 2021-05-20T08:57:54Z

@somangshu Issues observed so far:

By default Header is showing disabled for old tables
Extra rows are coming in the table for the below cases:
-Existing table in the preview
-If I create a table on release env and check on preview
~~3. First row height changes when all the header property is in disabled state~~

somangshu · 2021-05-21T11:08:03Z

By default Header is showing disabled for old tables

@riodeuno will need some help here. How do we migrate the old table for this case?

SatishGandham

Looks good except for the small change requested.

-- Use isHeaderVisible variable for conditional rendering

somangshu

Approve to run test

somangshu · 2021-06-04T11:25:59Z

@wmdev0808 some related tests are failing here! Can you check

wmdev0808 · 2021-06-04T14:46:16Z

@wmdev0808 some related tests are failing here! Can you check

Hi @somangshu, I've reviewed, but I don't think it is directly related to the current PR.
I can see a few failing cypress tests, but I can't get the point

somangshu · 2021-06-07T05:41:15Z

@wmdev0808 some test fails here because the tests cannot find the table header. Check this out.

@vicky-primathon can you take a look?

vicky-primathon · 2021-06-07T05:55:10Z

Sure @somangshu looking into it

vicky-primathon · 2021-06-07T06:35:49Z

@wmdev0808 @somangshu we are hiding table header section by default, this is causing table test cases to fail. Can we enable the table header toggles by default

somangshu · 2021-06-07T14:38:00Z

@vicky-primathon this needs migration I believe. For an old table widget, the header options are hidden by default.

Please help @wmdev0808 with this!

vicky-primathon · 2021-06-08T06:29:46Z

Okay @somangshu

-- Add a migration for the header options visibilities

vicky-primathon

Looks good to me

vivekverma2312 · 2021-06-10T09:02:04Z

Tested Deploy Preview and working fine. Good to merge into release.

somangshu · 2021-06-11T07:00:37Z

@yatinappsmith @NandanAnantharamu tests failing here! ForkApplication_spec.js is not related. The last release merge was two days ago.

wmdev0808 added 7 commits May 19, 2021 05:31

FEATURE-2111 : Show/Hide Table Controls

c76ef9e

-- Add the header options section for the visibility setting -- Render conditionally the header part based on the setting

FEATURE-2111 : Show/Hide Table Controls

f943d0b

-- Remove JS convertible -- Set isBindProperty and isTriggerProperty to false

FEATURE-2111 : Show/Hide Table Controls

35e28dc

-- PositionedContainer : Add padding-bottom: 42px in case of Table widget

FEATURE-2111 : Show/Hide Table Controls

7cf12ed

-- Remove extra space when the header options is all hidden

Revert "FEATURE-2111 : Show/Hide Table Controls"

9e0ec23

This reverts commit 1d3c2da.

FEATURE-2111 : Show/Hide table controls

c906b0e

-- Apply some cleanup

FEATURE-2111 : Show/Hide Table Controls

c1557c1

-- Adjust pageSize -- Change column header height -- Restore the logic for renderEmptyRow

wmdev0808 requested review from riodeuno and somangshu May 19, 2021 11:08

somangshu mentioned this pull request May 19, 2021

FEATURE-2111 : Show/Hide Table Controls #4551

Closed

7 tasks

appsmithorg deleted a comment from vivekverma2312 May 20, 2021

somangshu requested review from SatishGandham and removed request for riodeuno May 25, 2021 14:56

somangshu added the new-contribution label May 26, 2021

SatishGandham suggested changes May 27, 2021

View reviewed changes

Comment thread app/client/src/components/designSystems/appsmith/TableComponent/Table.tsx Outdated

somangshu removed their request for review June 1, 2021 12:07

wmdev0808 added 2 commits June 3, 2021 03:43

FEATURE-2111 : Show/Hide Table Controls

53fe96b

-- Use isHeaderVisible variable for conditional rendering

Merge branch 'release' into feature/2111-toggle-header-options

f7ae4f0

wmdev0808 requested a review from SatishGandham June 3, 2021 07:52

somangshu previously approved these changes Jun 3, 2021

View reviewed changes

SatishGandham previously approved these changes Jun 7, 2021

View reviewed changes

wmdev0808 added 2 commits June 9, 2021 06:16

FEATURE-2111 : Show/Hide Table Controls

8cef35c

-- Add a migration for the header options visibilities

Merge branch 'release' into feature/2111-toggle-header-options

a54b0ff

wmdev0808 dismissed stale reviews from SatishGandham and somangshu via a54b0ff June 9, 2021 10:20

wmdev0808 requested review from riodeuno, somangshu and vicky-primathon June 9, 2021 10:23

vicky-primathon approved these changes Jun 9, 2021

View reviewed changes

Merge branch 'release' into feature/2111-toggle-header-options

809416a

somangshu approved these changes Jun 14, 2021

View reviewed changes

riodeuno removed their request for review June 14, 2021 06:35

somangshu merged commit ca9324c into release Jun 15, 2021

somangshu deleted the feature/2111-toggle-header-options branch June 15, 2021 05:56

Uh oh!

Conversation

wmdev0808 commented May 19, 2021 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Checklist:

Test coverage results 🧪

Uh oh!

somangshu commented May 19, 2021

Uh oh!

somangshu commented May 19, 2021

Uh oh!

vivekverma2312 commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

somangshu commented May 21, 2021

Uh oh!

SatishGandham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

somangshu left a comment

Choose a reason for hiding this comment

Uh oh!

somangshu commented Jun 4, 2021

Uh oh!

wmdev0808 commented Jun 4, 2021

Uh oh!

somangshu commented Jun 7, 2021

Uh oh!

vicky-primathon commented Jun 7, 2021

Uh oh!

vicky-primathon commented Jun 7, 2021

Uh oh!

somangshu commented Jun 7, 2021

Uh oh!

vicky-primathon commented Jun 8, 2021

Uh oh!

vicky-primathon left a comment

Choose a reason for hiding this comment

Uh oh!

vivekverma2312 commented Jun 10, 2021

Uh oh!

somangshu commented Jun 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wmdev0808 commented May 19, 2021 •

edited by github-actions Bot

Loading

vivekverma2312 commented May 20, 2021 •

edited

Loading