feat: Support multiline inputs for select fields #953
Conversation
|
This pull request has been linked to Shortcut Story #40631: [Beam Enhancement] Allow SelectField to be use a multiline text field. |
bdow
force-pushed
the
sc-40631/multiline-select
branch
4 times, most recently
from
1def62f to
ba66a91
Compare
bdow
force-pushed
the
sc-40631/multiline-select
branch
from
ba66a91 to
a90f42c
Compare
| // If we do, we can infer whether we allow wrapping or not based on the PresentationProp. This will currently only be set for "rowHeight: 'fixed'" GridTable styles. | ||
| // Or, we can keep our existing default behavior, and allow the user to set `multiline = true` if they want to allow wrapping. | ||
| // If we change the default then we could have unexpected text wrapping in places like Filters. | ||
| const multilineProps = wrap ? { textAreaMinHeight: 0, multiline: true } : {}; |
There was a problem hiding this comment.
Question: Should this multiline: true use the multiline local variable? Not sure...
There was a problem hiding this comment.
multiline should always be true if we ever way the text to wrap. There are two conditions in which we want the text to wrap; 1. The caller defined multiline: true, or 2. PresentationContext.wrap is set to true.
In this case it could be that multiline is undefined, but the PresentationContext property is defined as true, so we'd want to still set multiline: true.
| ...otherProps | ||
| } = props; | ||
|
|
||
| const { wrap = multiline } = usePresentationContext(); |
There was a problem hiding this comment.
Kinda curious, seems like we have both wrap & multiline now...will they always be aligned, i.e. if wrap=true then multiline=true, and if wrap=false then multiline=false?
Wondering if like multiline=false happens on 73, then they had wrap=true set in PC, we'll end up with wrap=true but multiline=false ... is that okay/a supported configuration?
There was a problem hiding this comment.
Good question. I glazed over this a bit when doing it so will try to make this a bit simpler / easier to read.
There was a problem hiding this comment.
Alrighty, just went back over it and added a comment above a new variable allowWrap. This variable is going to be true if either ComboboxInputProps.multiline is true, or PresentationContext.wrap is true. i.e.
const allowWrap = wrap || multiline;
For now we'll always expect that if either is set, then we allow it to wrap. Maybe we'll eventually want the local multiline prop to override the presentation provider property if it is explicitly set. But I think for now this is fine.
…comments about ComboBoxInput wrap behavior
## [2.318.0](v2.317.2...v2.318.0) (2023-09-26) ### Features * Support multiline inputs for select fields ([#953](#953)) ([c2ef18f](c2ef18f))
|
🎉 This PR is included in version 2.318.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
| ...otherProps | ||
| } = props; | ||
|
|
||
| const { wrap = false } = usePresentationContext(); | ||
|
|
||
| // Allow the field to wrap whether the caller has explicitly set `multiline=true` or the `PresentationContext.wrap=true` |
SelectField and MultiSelectField will now support multiline inputs either via
multilineprop being set to true, or whenever thePresentationProvider.wrapprop is set totrue. E.g. When GridTable is rendered withrowHeight: "flexible"(default) then the SelectFields will support wrapping text.