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

Fix: Data Table rendering only in a small scrollable portion of the screen #3816

Merged
merged 3 commits into from
Apr 12, 2023

Conversation

curq
Copy link
Collaborator

@curq curq commented Apr 10, 2023

Description

Makes table component adjust its height on initial load. Before that it had fixed height on first load, resulting in table being rendered only in a small scrollable portion of the screen.

Issues Resolved

Fixes #3737

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Sirazh Gabdullin <sirazh.gabdullin@nu.edu.kz>
Signed-off-by: Sirazh Gabdullin <sirazh.gabdullin@nu.edu.kz>
@curq curq requested a review from a team as a code owner April 10, 2023 18:04
Signed-off-by: Sirazh Gabdullin <sirazh.gabdullin@nu.edu.kz>
@ananzh ananzh requested a review from joshuarrrr April 10, 2023 20:07
@codecov-commenter
Copy link

codecov-commenter commented Apr 10, 2023

Codecov Report

Merging #3816 (084e9aa) into main (a6af77d) will decrease coverage by 0.01%.
The diff coverage is n/a.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3816      +/-   ##
==========================================
- Coverage   66.42%   66.41%   -0.01%     
==========================================
  Files        3209     3209              
  Lines       61730    61730              
  Branches     9533     9533              
==========================================
- Hits        41004    41000       -4     
- Misses      18439    18442       +3     
- Partials     2287     2288       +1     
Flag Coverage Δ
Linux 66.36% <ø> (-0.01%) ⬇️
Windows 66.36% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@ashwin-pc ashwin-pc added the OSCI Open Source Contributor Initiative label Apr 10, 2023
@ananzh ananzh added the v2.7.0 label Apr 11, 2023
@manasvinibs
Copy link
Member

@curq Do you mind adding the repro steps for the issue? Before/after screenshot would be helpful too.

@joshuarrrr joshuarrrr self-assigned this Apr 11, 2023
@joshuarrrr joshuarrrr added the tableVis table visualization label Apr 11, 2023
@curq
Copy link
Collaborator Author

curq commented Apr 12, 2023

@manasvinibs The reproduction steps are described in the issue that this PR is aimed to fix. I copied the steps from there, you can always refer to this issue #3737
To Reproduce
Steps to reproduce the behavior:

  • Open any table visualisation split by rows with a default size of 5 -or- Go to this Sandbox Page
  • Inside the Split rows bucket, change the Size to 100
  • Click on Update. More rows should be shown but it should be paginated.
  • Click on Options in the side panel
  • Change Max rows per page to 100

Note that only 10 rows are shown, but the data table is actually scrollable with the mouse wheel to view 100 results within the small window:
It is also important to keep in mind that it only reproducible on initial load, and If we refresh the page or open it a new tab the bug is no longer visible.
before (screenshot from the issue):
image

after:
image

@manasvinibs
Copy link
Member

#3737

Thanks for the detailed steps. I'm able to repro the issue. Fix looks good to me.

@manasvinibs manasvinibs merged commit 416eb50 into opensearch-project:main Apr 12, 2023
@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-3816-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 416eb508b3b22fe81231b726dcfce09e3aad9bfb
# Push it to GitHub
git push --set-upstream origin backport/backport-3816-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3816-to-2.x.

@opensearch-trigger-bot
Copy link
Contributor

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-3816-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 416eb508b3b22fe81231b726dcfce09e3aad9bfb
# Push it to GitHub
git push --set-upstream origin backport/backport-3816-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3816-to-2.x.

@joshuarrrr
Copy link
Member

No need to backport - incorporated into #3397

sikhote pushed a commit to sikhote/OpenSearch-Dashboards that referenced this pull request Apr 24, 2023
…creen (opensearch-project#3816)

* Fix table not adjusting height on initial page load

Signed-off-by: Sirazh Gabdullin <sirazh.gabdullin@nu.edu.kz>

* formatting typo fix

Signed-off-by: Sirazh Gabdullin <sirazh.gabdullin@nu.edu.kz>

* Update CHANGELOG.md

Signed-off-by: Sirazh Gabdullin <sirazh.gabdullin@nu.edu.kz>

---------

Signed-off-by: Sirazh Gabdullin <sirazh.gabdullin@nu.edu.kz>
Signed-off-by: David Sinclair <david@sinclair.tech>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time-contributor OSCI Open Source Contributor Initiative tableVis table visualization v2.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Table visualisations sometimes show results in a small scrollable portion of the screen
6 participants