Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Prevent tabulator from overlapping when max_height is set #7403

Merged
merged 3 commits into from
Oct 15, 2024

Conversation

ahuang11
Copy link
Contributor

Closes #7401

Before:
image

After, properly truncates

import panel as pn
import pandas as pd
import numpy as np

pn.extension("tabulator")


df = pd.DataFrame(
    np.random.randn(100, 5),
)
pn.Column(pn.widgets.Tabulator(df, max_height=300), "Hello!").servable()
image
import panel as pn
import pandas as pd
import numpy as np

pn.extension("tabulator")


df = pd.DataFrame(
    np.random.randn(1, 5),
)
pn.Column(pn.widgets.Tabulator(df, max_height=1000), "Hello!").servable()

Does not use up all the space as max_height
image

@ahuang11 ahuang11 requested a review from philippjfr October 15, 2024 16:54
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 82.23%. Comparing base (46061e6) to head (d70d43b).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
panel/tests/ui/widgets/test_tabulator.py 85.71% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #7403       +/-   ##
===========================================
+ Coverage   71.86%   82.23%   +10.37%     
===========================================
  Files         336      337        +1     
  Lines       50542    50557       +15     
===========================================
+ Hits        36320    41574     +5254     
+ Misses      14222     8983     -5239     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@philippjfr
Copy link
Member

I think maxHeight is also a valid Tabulator configuration option. Maybe let's try that first.

@philippjfr
Copy link
Member

Also happy with this PR, but that might be simpler and clearer.

@ahuang11
Copy link
Contributor Author

ahuang11 commented Oct 15, 2024

Couldn't just dump it in config without a null check:

image

With null check, everything works as expected

serve_component(page, widget)

table = page.locator('.pnx-tabulator')
expect(table).not_to_have_css('max-height', '200px')
Copy link
Member

Choose a reason for hiding this comment

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

Seems like an odd test, why would I have max-height set to 200px in the first place? Seems like it should check that max-height is unset instead.

serve_component(page, widget)

table = page.locator('.pnx-tabulator')
expect(table).not_to_have_css('max-height', '200px')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay updated--seems counterintuitive to use to_have_css to check for null, but I guess that's the only way!

@ahuang11 ahuang11 requested a review from philippjfr October 15, 2024 18:16
@philippjfr philippjfr merged commit ced01e4 into main Oct 15, 2024
18 checks passed
@philippjfr philippjfr deleted the tabulator_max_height branch October 15, 2024 22:25
ahuang11 added a commit that referenced this pull request Oct 18, 2024
* limit tabulator height with max height

* use maxHeight config

* reverse logic
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tabulator may overlap with other components if max_height is set
2 participants