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

[Table Visualization] Fix width of multiple tables when rendered in column view #4638

Merged
merged 4 commits into from
Sep 13, 2023

Conversation

curq
Copy link
Collaborator

@curq curq commented Jul 28, 2023

Description

Fixes the visual bug where tables are rendered incorrectly as described in #4626
image
The cause of the issue was that this style

// Group container for table visualization components
.visTable__group {
flex: 0 0 auto;
}

Was not overriding the styles of OuiFlexItem, so the flex-grow remained default as 1, instead of 0. The fix sets the grow property to 0 through grow prop.

Issues Resolved

#4626

Screenshot

Here how the tables look after the fix.
image

Testing the changes

Go to tables visualizations, split rows then split tables. Choose the column view. Check if the visual bug is fixed.

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 <[email protected]>
Signed-off-by: Sirazh Gabdullin <[email protected]>
@codecov
Copy link

codecov bot commented Jul 28, 2023

Codecov Report

Merging #4638 (9fc3e77) into main (80fe14d) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #4638   +/-   ##
=======================================
  Coverage   66.49%   66.49%           
=======================================
  Files        3402     3402           
  Lines       65011    65011           
  Branches    10401    10401           
=======================================
  Hits        43230    43230           
  Misses      19212    19212           
  Partials     2569     2569           
Flag Coverage Δ
Linux_1 34.80% <100.00%> (ø)
Linux_2 55.31% <ø> (ø)
Linux_3 44.61% <ø> (+<0.01%) ⬆️
Linux_4 34.89% <ø> (ø)
Windows_1 34.81% <100.00%> (ø)
Windows_2 55.27% <ø> (ø)
Windows_3 ?
Windows_4 34.89% <ø> (ø)

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

Files Changed Coverage Δ
...le/public/components/table_vis_component_group.tsx 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

@@ -23,7 +23,7 @@ export const TableVisComponentGroup = memo(
return (
<>
{tableGroups.map(({ table, title }) => (
<EuiFlexItem key={title} className="visTable__group">
<EuiFlexItem key={title} className="visTable__group" grow={false}>
Copy link
Member

@manasvinibs manasvinibs Jul 28, 2023

Choose a reason for hiding this comment

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

Do we want to use flexGrow instead of grow here?

Copy link
Collaborator Author

@curq curq Jul 29, 2023

Choose a reason for hiding this comment

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

I believe we should use grow, as it is the prop of OuiFlexItem that sets the flex-grow css property of div element within to specific value.

ananzh
ananzh previously approved these changes Aug 21, 2023
@manasvinibs manasvinibs merged commit 17683e1 into opensearch-project:main Sep 13, 2023
63 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Sep 13, 2023
…olumn view (#4638)

* fix table vis bug

Signed-off-by: Sirazh Gabdullin <[email protected]>

* update CHANGELOG

Signed-off-by: Sirazh Gabdullin <[email protected]>

---------

Signed-off-by: Sirazh Gabdullin <[email protected]>
Co-authored-by: Anan Zhuang <[email protected]>
(cherry picked from commit 17683e1)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
ananzh added a commit that referenced this pull request Sep 13, 2023
…olumn view (#4638) (#4999)

* fix table vis bug
* update CHANGELOG

---------

Signed-off-by: Sirazh Gabdullin <[email protected]>
Co-authored-by: Anan Zhuang <[email protected]>
(cherry picked from commit 17683e1)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants