mirror of
https://github.com/ionic-team/ionic-framework.git
synced 2025-11-08 23:58:13 +08:00
fix(select): do not collapse to width: 0 when placed in flex container (#28631)
Issue number: Internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> We currently apply a workaround to `ion-select` so it can wrap correctly inside of `ion-item`:357b8b2beb/core/src/components/select/select.scss (L99-L103)However, this causes issues when a parent element has `display: flex` because the `ion-select` width becomes 0. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - In order to get the desired behavior, we need the `ion-select` (and other elements in the default slot) to either truncate or wrap within its own container and then have the entire container (i.e. the entire `ion-select`) wrap to the next line once the container is too small. To achieve this, I needed to set a min-width on `.item-inner` to define the point at which the element should wrap to the next line. I also changed the flex basis from `auto` to `0` which means the initial main size of the flex item will be 0px. In reality, this will be `--inner-min-width` since we also set `min-width: var(--inner-min-width)`. I used `0` for simplicity but I can change this to use the CSS variable if that's more clear. Since we also set `flex-grow: 1` we indicate that the element can grow from that basis (but it cannot shrink). I chose `--inner-min-width: 4rem` to minimize the number of diffs. We can certainly change this, but it may cause some diffs as certain elements will start wrapping sooner. I also chose to use `rem` because having a fixed min-width means that fewer characters are going to fit in the same space as text scales. I made this a CSS variable but left it undocumented. If developers need a way of changing this `min-width` they can request it and we can easily expose this variable. However, I think `4rem` is small enough that this should be sufficient. ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> The visual diffs here are correct. The table below shows the screenshot group and an explanation for why the changes are correct. | Path | Example | Details | | - | - | - | | `disabled` | [Link](https://github.com/ionic-team/ionic-framework/pull/28631/files#diff-d529716f95f7a7aa82c88588104220775b728af67077f48cd47a8afa04423143) | The searchbar is able to shrink slightly to fit on the same line as the checkbox at the bottom. | | `highlight` | [Link](https://github.com/ionic-team/ionic-framework/pull/28631/files#diff-0b64f24c91393923701d1ced4e330a1c6b926d72ee461b8ab1e135e708be3457) | We're changing how small the main content can get, so the input is only wrapping once it gets to `--inner-min-width`. | | `legacy/fill` | [Link](https://github.com/ionic-team/ionic-framework/pull/28631/files#diff-2ef8dbfa5e69e2b96c3e1ed29ab962f08cf5ba2aaf2af773e40bd143e38a4bef) | We're changing how small the main content can get, so the input is only wrapping once it gets to `--inner-min-width`. | | `slotted-inputs` | [Link](https://github.com/ionic-team/ionic-framework/pull/28631/files#diff-2f173c7303969d6a6c58f30a618cebc3caf918d3761fc83df5642fd48dfabd7b) | We're changing how small the main content can get, so the range is only wrapping once it gets to `--inner-min-width`. | `slotted-inputs` note: I'd argue many of these examples are not best practices. For example, adding a range in the start slot and the end slot is a bit unusual. I'm not aware of any native apps that implement this pattern. popover note: I [removed the `ion-item` from the `popover/test/async` test](331fcb859c). There was a diff because the min-width increased, but IMO that component should not be used in the popover test since we want to test the popover, not the item. -------- Demo: | `feature-7.6` | `branch` | | - | - | | <video src="https://github.com/ionic-team/ionic-framework/assets/2721089/693d4947-fa33-460d-bc7f-7b96b6338032"></video> | <video src="https://github.com/ionic-team/ionic-framework/assets/2721089/df35ca73-87aa-4e76-9bb7-99f0f2810640"></video> | (In this demo I updated the `ion-select` to wrap within its own container first instead of truncate. We may want to consider doing this by default, but I think this is out of scope for this task) --------- Co-authored-by: ionitron <hi@ionicframework.com> Co-authored-by: Brandy Carney <brandy@ionic.io>
This commit is contained in:
@ -40,9 +40,7 @@
|
||||
popover.addEventListener('ionMount', () => {
|
||||
popover.innerHTML = `
|
||||
<div style="padding: 10px;">
|
||||
<ion-list>
|
||||
<ion-item>Item 1</ion-item>
|
||||
</ion-list>
|
||||
Popover Content
|
||||
</div>
|
||||
`;
|
||||
});
|
||||
|
||||
Binary file not shown.
|
Before Width: | Height: | Size: 5.6 KiB After Width: | Height: | Size: 6.9 KiB |
Binary file not shown.
|
Before Width: | Height: | Size: 13 KiB After Width: | Height: | Size: 14 KiB |
Binary file not shown.
|
Before Width: | Height: | Size: 5.3 KiB After Width: | Height: | Size: 6.6 KiB |
Reference in New Issue
Block a user