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.

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

bug: item start and end slots wrap to next line #28769

Closed
3 tasks done
rdlabo opened this issue Jan 2, 2024 · 26 comments · Fixed by #28773
Closed
3 tasks done

bug: item start and end slots wrap to next line #28769

rdlabo opened this issue Jan 2, 2024 · 26 comments · Fixed by #28773
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@rdlabo
Copy link
Contributor

rdlabo commented Jan 2, 2024

Prerequisites

Ionic Framework Version

v7.x

Current Behavior

If an element other than ion-label is used in the default slot and the width is long, slot=end will break lines.

ex:

<ion-list>
  <ion-item>
    <ion-text style="background-color: lightgreen">
      A "Hello, World!" program is generally a simple computer program.
    </ion-text>
    <div slot="end" style="background-color: yellow">[slot=end]</div>
  </ion-item>
</ion-list>

スクリーンショット 2024-01-02 午後3 59 35

This has been occurring since v7.6.0 and does not occur in v7.5.8. In complex apps, ion-grid may be placed under ion-item, and this behavior can be a major problem.

Expected Behavior

Equivalent behavior to v7.5.8.

スクリーンショット 2024-01-02 午後3 58 56

Steps to Reproduce

  1. npm i @ionic/[email protected] or npm i @ionic/[email protected]

Code Reproduction URL

No response

Ionic Info

Ionic:

Ionic CLI : 7.1.1 (/opt/homebrew/lib/node_modules/@ionic/cli)
Ionic Framework : @ionic/angular 7.6.0
@angular-devkit/build-angular : 17.0.8
@angular-devkit/schematics : 17.0.8
@angular/cli : 17.0.8
@ionic/angular-toolkit : 9.0.0

Capacitor:

Capacitor CLI : 5.6.0
@capacitor/android : not installed
@capacitor/core : 5.6.0
@capacitor/ios : not installed

Utility:

cordova-res : 0.15.4
native-run : 2.0.0

System:

NodeJS : v20.10.0 (/usr/local/bin/node)
npm : 10.2.3
OS : macOS Unknown

Additional Information

No response

@ionitron-bot ionitron-bot bot added the holiday triage issues that were created during holiday period label Jan 2, 2024
Copy link

ionitron-bot bot commented Jan 2, 2024

Thanks for the issue! This issue has been labeled as holiday triage. With the winter holidays quickly approaching, much of the Ionic Team will soon be taking time off. During this time, issue triaging and PR review will be delayed until the team begins to return. After this period, we will work to ensure that all new issues are properly triaged and that new PRs are reviewed. In the meantime, please read our Winter Holiday Triage Guide for information on how to ensure that your issue is triaged correctly. Thank you!

@rdlabo rdlabo changed the title bug: the behavior of ion-item[slot=end] has changed from v7.6.0 bug(ion-item): the behavior of slot=end has changed from v7.6.0 Jan 2, 2024
@liamdebeasi
Copy link
Contributor

Thanks! I think we need to take a closer look at this behavior. While the text wrapping itself is correct, it looks like the end slot should not be pushed onto the next line when comparing with native iOS. I'll discuss the best course of action with the team.

@corradodellorusso
Copy link

corradodellorusso commented Jan 4, 2024

This also happens in ios mode, when you have a clickable ion-item with a ion-label with ion-text-nowrap class.
image

  <ion-item  lines="none" button>
      <ion-icon color="primary" name="reader-outline" size="large" slot="start"></ion-icon>
      <ion-label class="ion-text-nowrap">Some very long text!</ion-label>
  </ion-item>

@rdlabo rdlabo changed the title bug(ion-item): the behavior of slot=end has changed from v7.6.0 bug(ion-item): the behavior of slot has changed from v7.6.0 Jan 4, 2024
@lincolnthree
Copy link

We use ion-item for inputs, pickers, and other block level elements -- not just for text with buttons on the ends. This change has had ripple effects from our login screen to our main pages and lists.

I'm interested in testing a dev-build that fixes the end-wrapping behavior to see if it makes it possible for us to navigate the changes required to accommodate this new item layout spec. I think that fixing the end slot wrap should take care of most of it.

Also, any suggestions for a global CSS rule to make this work "the way it did before"?

@harry-molyneux
Copy link

I've found that this appears to be an issue with the flex property of the <div class="input-wrapper"> element. In previous versions of Ionic, I noticed that this element had flex: 1 1 0%;. If this style is applied, the issue appears to be resolved.

image

Therefore, I believe that the expected behaviour/fix is that the <div class="input-wrapper"> element within the shadow-root of an <ion-item> should have flex: 1 1 0%; if it contains an <ion-label> with class="ion-text-nowrap" (unless there would be any known side-effects of changing this style).

@lincolnthree
Copy link

I've found that this appears to be an issue with the flex property of the <div class="input-wrapper"> element.

Yeah, that's right. They changed this intentionally to support wrapping: ionic-team/ionic-docs#3313 (comment)

I think this should be a breaking / non-backwards compatible UI change, but that's just me. Or at least it should be done outside of the shadow-DOM, or with a CSS "off switch" so we can control how this behavior works globally in our app CSS.

@liamdebeasi
Copy link
Contributor

liamdebeasi commented Jan 10, 2024

Can everyone try the following dev build and let me know if it resolves the reported issue?

7.6.5-dev.11704916749.1e64a3a7

The start, default, and end slot containers will no longer wrap to new lines. However, content within each slotted container can still wrap.

CodePen sample: https://codepen.io/liamdebeasi/pen/xxBERbN?editors=1010

@liamdebeasi liamdebeasi changed the title bug(ion-item): the behavior of slot has changed from v7.6.0 bug: item start and end slots wrap to next line Jan 10, 2024
@liamdebeasi liamdebeasi added package: core @ionic/core package type: bug a confirmed bug report and removed holiday triage issues that were created during holiday period labels Jan 10, 2024
@harry-molyneux
Copy link

@liamdebeasi it is working perfectly for me with that dev build.

@rdlabo
Copy link
Contributor Author

rdlabo commented Jan 12, 2024

@liamdebeasi Thanks for your quick work. The slot=start problem is fixed, but not yet for ion-label[position=fixed].

#28780

7.6.5-dev.11704916749.1e64a3a7 work: https://codepen.io/rdlabo/pen/NWJbWRE?editors=1010

@lincolnthree
Copy link

@liamdebeasi This build works WAY better. I think we can probably resolve most of the issues with the start/end slots working like this.

@bobpeck
Copy link

bobpeck commented Jan 17, 2024

I tested 7.6.5-dev.11704916749.1e64a3a7 and 7.6.5 and the issue continues to persist.

<ion-item button detail="true">
  <ion-label class="ion-text-nowrap">
   Multi-line text that should ellipsis when it is too long to fit on one line. Lorem ipsum dolor sit amet, consectetur adipiscing elit.
  </ion-label>
</ion-item>

Screenshot 2024-01-17 at 2 07 42 PM

I'm struggling to come up with a css solution. The only temporary fix i can think of is to truncate the text in the template with a javascript splice. I also tried line clamp as an alternative to ion-text-nowrap, but the results were the same. the detail arrow goes to the next line.

@liamdebeasi
Copy link
Contributor

The slot=start problem is fixed, but not yet for ion-label[position=fixed].

Thanks for testing, I'll take a closer look.

I tested 7.6.5-dev.11704916749.1e64a3a7 and 7.6.5 and the issue continues to persist.

Thanks for testing. The chevron arrow icon isn't part of the start/end slots which is why my dev did not fix the issue. However, this is a very similar problem that we should fix too. I'll work on making that change.

@pete-mcwilliams
Copy link

pete-mcwilliams commented Jan 18, 2024

In 7.6.5 the end slot issue remains with the following

<ion-item>
  <ion-label class="ion-text-nowrap">
    Multi-line text that should ellipsis when it is too long to fit on one line.
    Lorem ipsum dolor sit amet, consectetur adipiscing elit.
  </ion-label>
  <div slot="end">Not end</div>
</ion-item>

Screenshot 2024-01-18 at 07 52 52

https://stackblitz.com/edit/angular-tcbg5u?file=src%2Fapp%2Fexample.component.html

@harry-molyneux
Copy link

I tested 7.6.5-dev.11704916749.1e64a3a7 and 7.6.5 and the issue continues to persist.

<ion-item button detail="true">
  <ion-label class="ion-text-nowrap">
   Multi-line text that should ellipsis when it is too long to fit on one line. Lorem ipsum dolor sit amet, consectetur adipiscing elit.
  </ion-label>
</ion-item>

@bobpeck , unless I am missing something it appears to be working perfectly for me on the dev build 7.6.5-dev.11704916749.1e64a3a7. See repo here: https://github.com/harry-molyneux/ionic-item-label-wrapping-bug

The second example on the form page is the same as yours and it is wrapping well with the detail arrow:

<ion-item button="true" detail="true">
    <ion-label class="ion-text-nowrap">
        Basic label with ion-text-nowrap: Multi-line text that should wrap when it is too long to fit on one line. Lorem ipsum dolor sit amet, consectetur adipiscing elit.
    </ion-label>
</ion-item>

image

Like I say, please let me know if I am missing something but this seems to be working well and I would like to see this made available ASAP. Thanks for your quick work @liamdebeasi

@bobpeck
Copy link

bobpeck commented Jan 18, 2024

@harry-molyneux @liamdebeasi apologies, when testing, i updated @ionic/core, not @ionic/angular. When testing @ionic/angular": "7.6.5-dev.11704916749.1e64a3a7 I see the expected behavior! Harry thank you for the check, and Liam, thank you for your work.

current test:

<ion-item button detail="true">
  <ion-label class="ion-text-nowrap">
    Multi-line text that should ellipsis when it is too long to fit on one line. Lorem ipsum dolor sit amet, consectetur adipiscing elit.
  </ion-label>
</ion-item>

Screenshot 2024-01-18 at 9 56 02 AM

@liamdebeasi
Copy link
Contributor

Here's a new dev build that should resolve the issue noted in #28769 (comment)

7.6.5-dev.11705617758.19ab0dd2

@aeharding
Copy link
Contributor

Another workaround I've been using is to add item-legacy class to IonItem (probably will go away in the future, but interim fix)

@fleboulch
Copy link

I have the same issue as #28769 (comment)

@liamdebeasi
Copy link
Contributor

@fleboulch Does the issue go away if you use the dev build?

@pete-mcwilliams
Copy link

The dev build fixes my issue. #28769 (comment)

@fleboulch
Copy link

The last dev build fixed the issue.
Thanks a lot @liamdebeasi for your amazing work! Have a good day

@sebastiencrevier
Copy link

When can we expect this fix to be released? I tried with the latest Ionic version 7.6.6 and it seems to have the same issue?

@liamdebeasi
Copy link
Contributor

The PR is still in review. Once it is merged, this issue will be closed.

github-merge-queue bot pushed a commit that referenced this issue Jan 26, 2024
Issue number: resolves #28769

---------

<!-- 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. -->

As part of #28146, we
allowed text wrapping inside of `ion-item` for accessibility purposes.
One of the behaviors we added was to allow start, default, and end
slotted containers to wrap to the next line to align with the iOS spec.
However, this decision was based on an incorrect assumption.

The following screenshot shows the Settings app on iOS:

| default scale | 310% scale |
| - | - |
| <img width="1170" alt="Frame 4"
src="https://github.com/ionic-team/ionic-framework/assets/2721089/462ef153-a060-41c8-9a00-f0aad17839be">
| <img width="1170" alt="Frame 5"
src="https://github.com/ionic-team/ionic-framework/assets/2721089/f047f880-7b80-4710-939b-96da075fbbf9">
|

At the default scale, the blue icon is in the iOS equivalent of the
"start" slot, "Bluetooth" is in the default slot, and "On" is in the
"end" slot. We incorrectly assumed the same markup was true when scaling
the text up. However, at 310% scale the icon, "Bluetooth" text, and "On"
text all become part of the default slot in a single container that
wraps. You can tell because the bottom border runs underneath the blue
icon at 310% whereas it does not at the default scale. This allows the
text to wrap underneath the blue icon. When we originally implemented
#28146 we thought that this meant the start, default, and end slot
containers should wrap to the next line.

I further validated this behavior by creating an app with Swift UI. I
created a list of items where each item has the native equivalent of a
checkbox in the `start` slot and multiple `ion-labels` in the default
slot of the item:

| Default Scale | 310% Scale |
| - | - |
|
![IMG_3133](https://github.com/ionic-team/ionic-framework/assets/2721089/8b55bd1d-f7a8-4fec-bda4-d1bb12f50d34)
|
![IMG_3134](https://github.com/ionic-team/ionic-framework/assets/2721089/92e8a196-36e4-47d6-a4e5-a0e991c78d0d)
|

The content within each label wraps within the container, but the
containers themselves never wrap to the next line.

Demo code:

```swift
import SwiftUI

struct ContentView: View {
    struct Item: Identifiable, Hashable {
        let id = UUID()
    }

    private var items = [
        Item(),
        Item(),
        Item(),
        Item(),
        Item()
    ]

    @State private var multiSelection = Set<UUID>()

    var body: some View {
        NavigationView {
            List(items, selection: $multiSelection) {_ in
                HStack {
                    Text("Column 1 with really long text")
                    Text("Column 2 with really long text")
                    Text("Column 3 with really long text")
                    Text("Column 4 with really long text")
                }
            }
            .toolbar { EditButton() }
        }
        Text("\(multiSelection.count) selections")
    }
}

#Preview {
    ContentView()
}
```

## What is the new behavior?
<!-- Please describe the behavior or changes that are being added by
this PR. -->

- This PR removes the ability for the start, default, and end slot
containers to wrap to the next line. This behavior aligns with
pre-v7.6.0 behaviors. The containers inside of the default slot will not
wrap to the next line. However, content within each container (such as
text within an `ion-label`) will continue to wrap to meet the team's
accessibility requirements.

## Does this introduce a breaking change?

- [ ] Yes
- [x] No

<!--
  If this introduces a breaking change:
1. Describe the impact and migration path for existing applications
below.
  2. Update the BREAKING.md file with the breaking change.
3. Add "BREAKING CHANGE: [...]" to the commit description when merging.
See
https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#footer
for more information.
-->


## Other information

<!-- Any other information that is important to this PR such as
screenshots of how the component looks before and after the change. -->

Dev build: `7.6.5-dev.11704916749.1e64a3a7`

---------

Co-authored-by: ionitron <[email protected]>
@liamdebeasi
Copy link
Contributor

This has been resolved via #28773, and a fix will be available in an upcoming release of Ionic Framework. We appreciate everyone's patience while we worked to resolve this issue. We realize this bug was disruptive, and we will try to be more mindful of this in the future. Let me know if you have any questions. Thanks!

@langy
Copy link

langy commented Feb 1, 2024

Thanks for the fix!

Copy link

ionitron-bot bot commented Mar 2, 2024

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Mar 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Development

Successfully merging a pull request may close this issue.