-
Notifications
You must be signed in to change notification settings - Fork 14
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
feat: allow overflow by default on ui.panel #896
base: main
Are you sure you want to change the base?
Conversation
// classes can be used for deephaven ui specific css | ||
// "deephaven.ui.components.Grid" -> "dh-grid" | ||
if (shouldAddClassName.has(newElement[ELEMENT_KEY])) { | ||
props.UNSAFE_className = `${props.UNSAFE_className ?? ''} ${`dh-${ | ||
newElement[ELEMENT_KEY].split('.').pop()?.toLowerCase() ?? '' | ||
}`}`.trim(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should just make Grid.tsx
and Flex.tsx
components instead that handle this
The shouldWrapTextChildren
also probably belongs in wrapper components instead of this file. There's not really any reason we should be doing any manipulation here IMO. It hides what actually happens with the components
The contextualHelp
is an exception I guess since we're wanting any prop called contextualHelp
to be modified
Fixes #178
This improves the default layout experience across a number of layout cases. The primary goal was to allow overflow auto to happen on ui.panel by default while still filling space when passing multiple objects such as tables and plots.
Tested across the cases in ui_flex.py, added e2e tests for those cases.
Changes are as follows:
BREAKING CHANGE: ui.panel now has overflow auto set by default and ui.grid now has default padding