Skip to content

mgr/dashboard: Prevent table items from getting selected while expanding#37482

Merged
LenzGr merged 1 commit intoceph:masterfrom
rhcs-dashboard:datatable_click_prevent
Oct 30, 2020
Merged

mgr/dashboard: Prevent table items from getting selected while expanding#37482
LenzGr merged 1 commit intoceph:masterfrom
rhcs-dashboard:datatable_click_prevent

Conversation

@nizamial09
Copy link
Copy Markdown
Member

@nizamial09 nizamial09 commented Sep 29, 2020

While clicking on the expand button inside a table, the entire row is getting selected. This commit prevent that from happening.

captured (1)

Fixes: https://tracker.ceph.com/issues/47376
Signed-off-by: Nizamudeen A nia@redhat.com

Checklist

  • References tracker ticket
  • Updates documentation if necessary
  • Includes tests for new functionality or reproducer for bug

Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox

@nizamial09 nizamial09 requested a review from a team as a code owner September 29, 2020 19:51
@nizamial09 nizamial09 added bug-fix dashboard needs-review skip-teuthology For PRs whose changes do not have an effect on QA runs/changes are not being tested in Teuthology labels Sep 29, 2020
Copy link
Copy Markdown
Contributor

@tspmelo tspmelo left a comment

Choose a reason for hiding this comment

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

Please add a new unit test to check if the selection is not changed when expanding a different row.

Copy link
Copy Markdown
Member

@epuertat epuertat left a comment

Choose a reason for hiding this comment

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

Once addressed @tspmelo's comments, I'm ok with this PR! Thanks @nizamial09!

@nizamial09 nizamial09 force-pushed the datatable_click_prevent branch from bedbe4e to 2cfebf6 Compare October 1, 2020 08:30
@nizamial09 nizamial09 requested a review from tspmelo October 1, 2020 08:32
@nizamial09
Copy link
Copy Markdown
Member Author

nizamial09 commented Oct 1, 2020

Please add a new unit test to check if the selection is not changed when expanding a different row.

Added the unit test. Please take a look, thanks. @tspmelo

Just a suggestion, can you also update the gif in PR description which covers this test?

Copy link
Copy Markdown
Contributor

@avanthakkar avanthakkar left a comment

Choose a reason for hiding this comment

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

Tested it locally, LGTM!

@nizamial09 nizamial09 force-pushed the datatable_click_prevent branch from 2cfebf6 to 65c7914 Compare October 1, 2020 10:22
@nizamial09 nizamial09 requested a review from tspmelo October 1, 2020 10:22
@nizamial09
Copy link
Copy Markdown
Member Author

@avanthakkar Just saw your suggestion. The suggestion was edited into my comment so I missed it before. Anyway I've updated the description with the new gif. Thanks

@avanthakkar
Copy link
Copy Markdown
Contributor

@avanthakkar Just saw your suggestion. The suggestion was edited into my comment so I missed it before. Anyway I've updated the description with the new gif. Thanks

Yeah, I just saw that, my mistake. Thanks!

Copy link
Copy Markdown
Member

@votdev votdev left a comment

Choose a reason for hiding this comment

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

I think you should check if e2e test aren't broken by this PR.

  Configuration page
    breadcrumb test
      ✓ should open and show breadcrumb (2709ms)
    fields check
      1) should verify that selected footer increases when an entry is clicked
      ✓ should check that details table opens (w/o tab header) (1077ms)
    edit configuration test
      ✓ should click and edit a configuration and results should appear in the table (9299ms)
      ✓ should show only modified configurations (1439ms)
      ✓ should hide all modified configurations (1457ms)


  5 passing (27s)
  1 failing

  1) Configuration page
       fields check
         should verify that selected footer increases when an entry is clicked:
     AssertionError: expected 0 to equal 1
      at Context.eval (https://bionic-huge--734e7c1f-b7d8-454f-a114-e1abf5a3d023:41140/__cypress/tests?p=cypress/integration/cluster/configuration.e2e-spec.ts:21:53)

@nizamial09
Copy link
Copy Markdown
Member Author

I think you should check if e2e test aren't broken by this PR.

  Configuration page
    breadcrumb test
      ✓ should open and show breadcrumb (2709ms)
    fields check
      1) should verify that selected footer increases when an entry is clicked
      ✓ should check that details table opens (w/o tab header) (1077ms)
    edit configuration test
      ✓ should click and edit a configuration and results should appear in the table (9299ms)
      ✓ should show only modified configurations (1439ms)
      ✓ should hide all modified configurations (1457ms)


  5 passing (27s)
  1 failing

  1) Configuration page
       fields check
         should verify that selected footer increases when an entry is clicked:
     AssertionError: expected 0 to equal 1
      at Context.eval (https://bionic-huge--734e7c1f-b7d8-454f-a114-e1abf5a3d023:41140/__cypress/tests?p=cypress/integration/cluster/configuration.e2e-spec.ts:21:53)

Oh yeah, thanks. Let me check this locally.

@nizamial09 nizamial09 force-pushed the datatable_click_prevent branch from 65c7914 to 978beeb Compare October 18, 2020 14:15
@nizamial09
Copy link
Copy Markdown
Member Author

jenkins test dashboard

1 similar comment
@nizamial09
Copy link
Copy Markdown
Member Author

jenkins test dashboard

While clicking on the expand button inside a table, the entire row is getting selected. This commit prevent that from happening.

Fixes: https://tracker.ceph.com/issues/47376
Signed-off-by: Nizamudeen A <nia@redhat.com>
@nizamial09 nizamial09 force-pushed the datatable_click_prevent branch from 978beeb to baf1705 Compare October 27, 2020 10:58
@nizamial09
Copy link
Copy Markdown
Member Author

jenkins test make check

@nizamial09 nizamial09 requested a review from votdev October 27, 2020 13:40
@nizamial09
Copy link
Copy Markdown
Member Author

@votdev I removed certain lines from the e2e files like should verify that selected footer increases when an entry is clicked. IMO this feels like a unit test and similar unit test is already covered in this PR (row getting selected).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug-fix dashboard skip-teuthology For PRs whose changes do not have an effect on QA runs/changes are not being tested in Teuthology

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants