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.

Sign up for GitHub

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

Jump to bottom

fix: Tab Panels contents should take up the full height #340

Merged
merged 3 commits into from
Mar 5, 2024
Merged

fix: Tab Panels contents should take up the full height #340

merged 3 commits into from
Mar 5, 2024

Conversation

mofojed
Copy link
Member

@mofojed mofojed commented Mar 4, 2024

@mofojed mofojed requested a review from bmingles March 4, 2024 21:04
@mofojed mofojed self-assigned this Mar 4, 2024
bmingles
bmingles requested changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

Left a suggestion for using display:flex instead of 100% height

@mofojed mofojed requested a review from bmingles March 5, 2024 17:30
@mofojed mofojed enabled auto-merge (squash) March 5, 2024 17:32
bmingles
bmingles requested changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@bmingles bmingles 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 this needs to be rebased to work. The Item mapping that I added in my PR doesn't seem to be present which is throwing errors.

mofojed added 3 commits March 5, 2024 14:34
@mofojed
fix: Tab Panels contents should take up the full height
ed69500 
- Fixes deephaven#175
- Tested using the snippet from within the ticket, as well as some other examples with tab_panels in vertical orientation
@mofojed
Add some unit tests to ensure the height attribute gets set
5f7c0a6
@mofojed
Clean up based on review
c058a39 
- Just change the style to `display: flex`. Much cleaner.
@mofojed mofojed requested a review from bmingles March 5, 2024 19:37
bmingles
bmingles approved these changes Mar 5, 2024
View reviewed changes
Copy link
Contributor

@bmingles bmingles left a comment

Choose a reason for hiding this comment

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

Tested locally and height fills the container now.

@mofojed mofojed merged commit 6028195 into deephaven:main Mar 5, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmingles bmingles bmingles approved these changes

Assignees

@mofojed mofojed

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

View in ui.tab_panels do not grow to fill content automatically
2 participants
@mofojed @bmingles