8000 fix(select): do not collapse to width: 0 when placed in flex containe… · ionic-team/ionic-framework@e71e7a0 · GitHub
[go: up one dir, main page]

Skip to content

Commit e71e7a0

Browse files
liamdebeasiIonitronbrandyscarney
authored
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`: https://github.com/ionic-team/ionic-framework/blob/357b8b2beb29b95d53ef043af349067be1d32658/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](331fcb8). 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>
1 parent 8c235fd commit e71e7a0

File tree

46 files changed

+30
-24
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+30
-24
lines changed

core/src/components/item/item.scss

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,15 @@
5151
* @prop --highlight-color-valid: The color of the highlight on the item when valid. Only applies to inputs and textareas using the legacy form syntax. DEPRECATED: Highlights can be styled on `ion-input` or `ion-textarea` when using the modern form syntax.
5252
* @prop --highlight-color-invalid: The color of the highlight on the item when invalid. Only applies to inputs and textareas using the legacy form syntax. DEPRECATED: Highlights can be styled on `ion-input` or `ion-textarea` when using the modern form syntax.
5353
*/
54+
55+
/**
56+
* We change the minimum width as the
57+
* font size changes. Using a fixed minimum
58+
* width means that fewer and fewer characters
59+
* can be displayed in the same space as the
60+
* text grows.
61+
*/
62+
--inner-min-width: 4rem;
5463
--border-radius: 0px;
5564
--border-width: 0px;
5665
--border-style: solid;
@@ -80,15 +89,6 @@
8089

8190
position: relative;
8291

83-
// When an item containing a select is inside of a
84-
// flex container the item will collapse to 0px
85-
// width due to the select setting the width to 0px.
86-
// By setting the flex property here, we are
87-
// allowing the item to grow to fill the flex container.
88-
// If the item is inside of a block container this
89-
// property will be ignored.
90-
flex: 1;
91-
9292
align-items: center;
9393
justify-content: space-between;
9494

@@ -310,7 +310,7 @@ button, a {
310310
// This flex property is required in order to make
311311
// the elements wrap when there is a slotted start
312312
// element and a label
313-
flex: 1 0 auto;
313+
flex: 1 0 0;
314314

315315
flex-direction: inherit;
316316

@@ -322,6 +322,15 @@ button, a {
322322
align-items: inherit;
323323
align-self: stretch;
324324

325+
/**
326+
* The min-width defines when the
327+
* content in the default slot should
328+
* stop wrapping/truncating within its own
329+
* container. At this point the entire
330+
* container will wrap to the next line.
331+
*/
332+
min-width: var(--inner-min-width);
333+
325334
// Max width must be set to 100%, otherwise the
326335
// elements will overflow this container instead
327336
// of wrapping

0 commit comments

Comments
 (0)
0