ionic-team
feat: improve performance when using ion-item-sliding
Feature Request
Ionic version:
[x] 5.x
Describe the Feature Request
When rendering lists of items with complex structures including ion-item-sliding and ion-item-options, DOM rendering can be incredibly expensive. This is exacerbated on mobile devices with weaker G/CPUs and less memory where rendering performance is generally slow.
It is not currently a "supported feature" to show the contents of items dynamically when sliding begins, then hide them again when sliding ends, to keep DOM node count and initial render under control.
Additionally, this makes it difficult to create custom components that use ion-item-sliding and insert slotted content into ion-item-options elements, or even insert their own, conditionally via ng-content or a similar composition technique.
There are a few problems that get in the way of this:
- Lack of observable or event that would notify strictly on item is open or not (boolean). This can be done using the
(ionDrag)event, but it's a bit painful. - Width of
options-start/endis calculated at initial render time. If items are added/removed conditionally from these slots using*ngIfor the like, the window for which the sliding item will open does not adjust its width. Therefore when the item is slid open, then elements are rendered (fade in animation to make it nice), the item will snap closed again because it does not believe there is anything to show. - The sliding animation is jittery on many devices (seems mostly chromium based issue) which can lead to strange gestures and things snapping closed again while dragging.
Describe Preferred Solution
ion-item-sliding should:
- Publish an observable or event emitter that simply reports "am I open or not".
- Width of items should be calculated before and after
(ionDrag)has been started. If new elements have been added, the width should be adjusted. AContentObservercomes to mind so that a fixed-width pre-rendered placeholder is not required.
Describe Alternatives
Basically hack this in and implement it manually, which is complex and difficult to get right due to the need to set a fixed width to a placeholder element inside the ion-item-options component so that once sliding starts, the item will know it has contents, set a width, and stay open/not snap closed again.
Related Code
<ion-item-options *ngIf="state.isSlidingEnabled" [hidden]=disabled #optionsEnd class="animate-fade-in">
<ng-container slot="options-end">
<ng-container *ngIf="state.sliderOpen; else placeholderTemplate">
<ng-container *ngIf="sliderOpen">
<!-- Actual contents we want dynamically rendered -->
<ion-item-option color="transparent"
(tap)="item.toggleSlider()">
<ion-icon slot=top name="chevron-forward">
</ion-icon>
</ion-item-option>
</ng-container>
<ng-template #placeholderTemplate>
<div style="width: 185px"> <!-- Fixed width placeholder -->
</div>
</ng-template>
...
</ion-item-options>
Additional Context
Thanks for the issue. Can you provide a GitHub repo that shows the performance issue as it is right now?
@liamdebeasi This issue isn't about the performance issue. It's more about conditional rendering. Trying to keep them separate, but wanted to explain some of the motivation for this. This one is an issue even if you just want to conditionally render things inside sliding item options. But I'll see what I can do when I get a minute.
Thanks for the follow up. I guess I am confused as the first post calls out performance as a motivation for this issue in the first paragraph, so I am trying to understand how these issues appear in applications.
@liamdebeasi Fair, it is performance motivated - that was the initial reason why I implemented this workaround a few months back - and am now revisiting it as I'm trying to address major perf issues on mobile devices. I'd certainly love to just shove all the elements on screen at once and have it work out. But the issue is still a problem when you want to add/remove things from ion-item-options elements conditionally. I understand the need for a repro and will do my best.
Also not intending to be... err... mansplaining, I guess? I know you're an expert in this stuff. Just trying to explain a bit what I think is going on.
Counting the DOM:
ion-item-sliding: 1 to 3 (depending if you have no options, or options on both sides). This one is lean.ion-item: 11 elements just for slots and item-inner/wrapperion-item-option/ion-button: 15 at a minimum, each.
Each <ion-button> element added to a sliding item can result in upwards of 15-20 DOM elements *per ion-item-option.
So adding this up. We have close to 30 DOM elements just to show the skeleton of an ion-item-sliding with one option. This is OK so far, well within reason.
Now let's make it 5 options (not uncommon). Now each item requires ~90 DOM elements. Now add your own content to the item. Call it 100 to account for a few ion-labels and ion-icons in the item itself, maybe a '...' button at the end.
Let's say we have a list of 100 items. At 100 elements per item, that means 10,000 DOM elements that need to be created all at once just to show the initial render.
On a mobile device that can be crushing. 7500 of these elements could be conditionally rendered only as needed - if item options allowed it. 100 items in a list is not uncommon, and shouldn't require virtual scrolling. Even 500 list items should be... sort of reasonable, you could incrementally load it. But that's just my current line of thinking of why this is a perf issue.
@liamdebeasi PS. Not sure if I'm missing it or if it's still a thing, but I don't think the stackblitz repos have been updated for Ionic 5.
@liamdebeasi I updated the repro with a reload button to make things a bit more visible. Feel free to play with the length of the list, I just set it at 500, which is slow even on a very beefy desktop. Even 100 takes a noticeable time to render. Then remove the options and try again.
And just for emphasis. I actually am surprised the DOM node count is this high: A list of 500 items.
@liamdebeasi Also just added a toggle for options on/off so you can more easily benchmark.
Putting this here because I don't trust StackBlitz:
<ion-app>
<ion-header>
<ion-toolbar>
<ion-buttons slot=start>
<ion-button slot=end fill=clear (click)="reload(true)">
<ion-toggle (ionChange)="toggle()" [checked]="optionsEnabled"></ion-toggle>
</ion-button>
</ion-buttons>
<ion-title>List of Sliding Items</ion-title>
<ion-buttons slot=end>
<ion-button slot=end fill=clear (click)="reload(true)">
<ion-icon name="refresh-outline"></ion-icon>
</ion-button>
</ion-buttons>
</ion-toolbar>
</ion-header>
<ion-content>
<ion-list>
<ion-item-sliding *ngFor="let i of items">
<ion-item-options side=start>
<ng-container *ngIf="optionsEnabled">
<ion-item-option slot=start color="primary">
<ion-icon slot=top name="chevron-back">
</ion-icon>
</ion-item-option>
<ion-item-option color="secondary">
<ion-icon slot=top name="chevron-forward">
</ion-icon>
</ion-item-option>
<ion-item-option color="tertiary">
<ion-icon slot=top name="chevron-up">
</ion-icon>
</ion-item-option>
</ng-container>
</ion-item-options>
<ion-item [button]=true>
<ion-label>{{i}}</ion-label>
<ion-button slot=end fill=clear>
<ion-icon name="ellipsis-vertical"></ion-icon>
</ion-button>
</ion-item>
<ion-item-options side=end>
<ng-container *ngIf="optionsEnabled">
<ion-item-option color="primary">
<ion-icon slot=top name="chevron-back">
</ion-icon>
</ion-item-option>
<ion-item-option color="secondary">
<ion-icon slot=top name="chevron-forward">
</ion-icon>
</ion-item-option>
</ng-container>
</ion-item-options>
</ion-item-sliding>
</ion-list>
</ion-content>
</ion-app>
import { ChangeDetectorRef, Component } from "@angular/core";
import { AlertController } from "@ionic/angular";
@Component({
selector: "my-app",
templateUrl: "./app.component.html",
styleUrls: ["./app.component.css"]
})
export class AppComponent {
optionsEnabled = true;
items = [];
constructor(private _cdr: ChangeDetectorRef) {
this._cdr.detach();
}
ngAfterViewInit() {
this.reload(true);
}
toggle() {
this.optionsEnabled = !this.optionsEnabled;
this._cdr.detectChanges();
}
reload(detect?: boolean) {
this.items = null;
if (detect) {
this._cdr.detectChanges();
}
window.setTimeout(() => {
this.items = new Array(500);
for (var i = 0; i < this.items.length; i++) {
this.items[i] = i;
}
if (detect) {
this._cdr.detectChanges();
}
});
}
}
Thanks for the additional information, that was very helpful. I can reproduce the behavior your are describing. I have not dug too much into this yet, but there seem to be a few factors that contribute to this performance issue:
- Angular: I see a lot of Angular-specific code running in a Timeline test on Chrome dev tools. Reproducing this issue outside of Angular does reduce the delays, though there is definitely more in Ionic/Stencil world that we need to improve.
ion-item-slidinggesture and setup code: There is a lot of JavaScript that we currently run when creating an instance ofion-item-sliding. In particular, code related to the gesture and computing values seem to be the most costly here. We do have plans to refactor this as part of https://github.com/ionic-team/ionic-framework/issues/22909, so that would be a good opportunity to address any performance issues there as well.- Number of DOM elements: As part of our refactor, we should also look to reduce the number of DOM elements rendered as part of this component usage. We likely will not be able to reduce the number of elements used for components like
ion-itemorion-button, but streamliningion-item-optionsandion-item-slidingis something we are definitely interested in.
I will keep this issue open for now, but will re-evaluate when we get around to tackling https://github.com/ionic-team/ionic-framework/issues/22909.
PS. Not sure if I'm missing it or if it's still a thing, but I don't think the stackblitz repos have been updated for Ionic 5.
This is on our to-do list for our Framework v5.7.0 work, which is the next minor release of Framework v5: https://github.com/ionic-team/ionic-framework/milestone/101.
Forgot to add this, but does that pretty much cover the issues you are seeing? Is there anything I missed?
Thanks @liamdebeasi -- Partly. This mostly covers the performance related aspects regarding Stencil/Ionic - that part looks good.
However I am not sure it addresses the actual feature requested my original description, which is the ability to only render the contents of ion-item-options when the item is opened on the respective side. I am also not sure it addresses the ability to add options/components while the slider is open/opening (properly resizing).
Regardless of how efficient you get the stencil code to be... we still either need conditional rendering of ion-item-options or the ability to do it ourselves so that what we put into the slider can be handled efficiently as well.
https://user-images.githubusercontent.com/362329/113752293-c8c01300-96da-11eb-8926-85ad2234fb70.mov
I think this issue should probably be split up. One for the features I requested (this one or whichever you feel is most appropriate given this one really shifted toward perf), and one for the performance problems, which is what I attempted (and likely failed to do) by referencing the original performance issue: https://github.com/ionic-team/ionic-framework/issues/23116 -- does that make sense?
Assuming we resolve most of the outstanding performance issues with ion-item-sliding, what would the use case for this feature be? As far as I am aware this kind of functionality does not exist in the native sliding component on iOS. When the sliding options are not revealed, we hide them using display: none so there should not be many performance issues related to these.
I guess another way to think about this is where most of the performance issues are coming from ion-item-sliding, being able to remove ion-item-option elements from the DOM until they are ready to be shown likely will not make much of a difference. Another thing to consider is would manually removing/adding them to the DOM be creating more work for the browser as opposed to just leaving them in the DOM and hiding them with display: none.
@liamdebeasi In general, the use case is I am putting something heavy into the ion-sliding-options (not an ion-item-option) that I don't want to render for each item unless the user interacts with that specific item.
Without the ability to defer this rendering, it will cause the same performance problems that you'll be spending time to fix. I am not familiar with the native iOS slider component, but you guys have the feature so we're using it :) Rendering 100 elements when a specific item is slid open is a lot better than rendering 10,000 elements when the whole list shows up. It's the same issue.
The only features we need to support this are more responsive sizing of the ion-item-options in the sliders, and an event to let us know if the options are visible at all or not. I've implemented this myself using the (ionDrag) event, but I still can't get the slider to respect the new size of the options. (Thus the need for a placeholder as I mentioned above.)
Also for the record, we are using this component for all platforms. Not just iOS.
<ng-container slot="options-end" *ngIf="state.editing">
<ng-container *ngIf="viewModel.options$ | push as options">
<ng-container *rxLet="cardItem.sliderOpen$ as sliderOpen">
<div *ngIf="!sliderOpen; else sliderOpenTemplate"
style="width: 185px">
</div>
<ng-template #sliderOpenTemplate>
<ion-item-option color="transparent"
(tap)="cardItem.toggleSlider()">
<ion-icon slot=top name="chevron-forward"></ion-icon>
</ion-item-option>
<ion-item-option color=secondary
[disabled]="!options.canMoveMain"
(tap)="actions.moveCard($event, deck, options.otherZone, 'main', card, 1)">
Main
<ion-icon slot=top name="arrow-back"></ion-icon>
</ion-item-option>
<ion-item-option color=secondary
[disabled]="!options.canMoveOther"
(tap)="actions.moveCard($event, deck, 'main', options.otherZone, card, 1)">
{{ options.otherZoneName }}
<ion-icon slot=top name="arrow-forward"></ion-icon>
</ion-item-option>
<ion-item-option (tap)="showCardMenu($event, deck, card)">
<ion-icon slot=top ios=ellipsis-horizontal
md=ellipsis-vertical>
</ion-icon>
</ion-item-option>
</ng-template>
</ng-container>
</ng-container>
</ng-container>
public slidingDisabledEnd$ = new BehaviorSubject(false);
public slidingDisabled$: Observable<boolean> = combineLatest([this.slidingDisabledStart$, this.slidingDisabledEnd$])
.pipe(map(([start, end]) => start && end));
private _sliderMoved$ = new BehaviorSubject(0);
private sliderMoved$ = this._sliderMoved$
.pipe(distinctUntilChanged())
.pipe(throttleTime(500, asyncScheduler, { leading: true, trailing: true }));
public sliderOpen$: Observable<boolean> = this.sliderMoved$
.pipe(switchMap(() => from(this.slider?.getSlidingRatio() ?? Promise.resolve(0))))
.pipe(debounce(ratio => ratio > 0 ? of(0) : interval(500)))
.pipe(map(ratio => ratio !== 0))
.pipe(shareReplay({ refCount: true, bufferSize: 1 }));
Regarding knowing which side is open, we already have a feature request open for this: https://github.com/ionic-team/ionic-framework/issues/18533
Sorry if this was already mentioned somewhere, but can you clarify what "responsive sizing" means in the context of ion-item-options?
@liamdebeasi Ah yeah! Sorry, watch the video clip I attached above. It demonstrates adding an element while the slider is in motion. The size isn't correctly computed until the slider is opened, then closed, then reopened quickly (in my impl. because I have a debounce on the state change, see code snippit just above).
I guess another way to think about this is where most of the performance issues are coming from ion-item-sliding, being able to remove ion-item-option elements from the DOM until they are ready to be shown likely will not make much of a difference. Another thing to consider is would manually removing/adding them to the DOM be creating more work for the browser as opposed to just leaving them in the DOM and hiding them with display: none.
Maybe. I'm not 100% convinced. There's a lot of purple (rendering) in my benchmarks too. But I am not intimate enough with the browser rendering process to know the cost difference between display: none and not rendered at all. If the JS performance-hit is slowing down rendering that much I could see that.
Regardless, let's assume there is a heavy Angular, Style, or other JavaScript cost to instantiating the user's components too. The core issue remains unchanged I believe. Maybe there are images in there, for example. That shouldn't be loaded until slid open... etc etc. Simply having images in the slider (think SVGs) would bottleneck the HTTP request performance for the browser as it resolves URLs.
PS. FWIW our users love the ability to slide items open. It's a great feature, heavily used even on Desktop.
@liamdebeasi In general though, when rendering many things -- assuming loops are the bane of performance -- isn't it best practice to render as little as possible until necessary (or give the option to), unless in specific instances where rendering batches of items in the same tick/frame prevents exploding layout recalcs?
Or am I missing something?
PS. This is a video of the whole hodgepodge put together, using a placeholder to force the width, and then fading the elements in, on first interaction:
I think it's pretty seamless and prevents pre-rendering every element under the sliding item :)
https://user-images.githubusercontent.com/362329/113757649-d9738780-96e0-11eb-98c8-f9cb12aa1e17.mov
OH. I forgot to mention another critical issue that really requires this feature... these items are transparent to support the theme background gradient :D
If you pre-render the contents of the item-sliding (or forget to set them to opacity: 0), it breaks everything because they bleed through. So you need to be able to toggle the opacity or something to that effect anyway. So that's yet another use-case for having the triggers feature you just linked. But it relates to this because this can easily done using dynamic rendering and solves two problems with one solution.
You could even start rendering the options when the user puts their finger on the item, before they even start a swipe, to prevent any appearance of "nothing here" rendering jank, if a fade is undesirable.
Ah yeah! Sorry, watch the video clip I attached above. It demonstrates adding an element while the slider is in motion. The size isn't correctly computed until the slider is opened, then closed, then reopened quickly (in my impl. because I have a debounce on the state change, see code snippit just above).
Ok that makes sense, thanks.
In general though, when rendering many things -- assuming loops are the bane of performance -- isn't it best practice to render as little as possible until necessary (or give the option to), unless in specific instances where rendering batches of items in the same tick/frame prevents exploding layout recalcs?
Yes however the biggest hits to performance will be rendering ion-item-sliding given what we need to do to compute dimensions and get the gesture setup. Omitting a few buttons likely will not make a big difference. I guess what I am saying is if we address the main performance issues, removing these option buttons from the DOM ideally would not even be necessary and would just be creating more work for yourself. However, I won't be able to make that assessment until the initial performance issues are resolved.
We plan on refactoring the ion-item-sliding component with desktop + performance improvements as part of our Framework v6 work which is already underway. I am not saying "no" to the other improvements noted in this thread, I am just saying that the team here needs to work on the performance improvements first before we can accurately determine what other work needs to happen. I will keep this issue open for now and will update this thread when we have completed work on the updates. Thanks!
@liamdebeasi That makes sense. And I hope you're right that it won't make a difference to have a "few" extra buttons. I'm just doing the math in my head and those buttons make a lot of nodes :) As long as you don't break what I've done ;) Haha! (I kid, I kid)
Looking forward to seeing what you can come up with. Until then, I hope this has been helpful at least. Your great work and valuable thoughts are always appreciated over here.
Oh one last thought regarding this:
the biggest hits to performance will be rendering ion-item-sliding given what we need to do to compute dimensions and get the gesture setup
Any way to defer gesture initialization/dimensions until the finger hits the element? (Obviously you need a listener of some kind to catch the initial & subsequent events) Just spitballing here. Could be the ultimate optimization but likely not doable, hah!
One actual last thought (hopefully) I'll leave here -- since this issue has kept me up for a few weeks and I just can't seem to get it off my mind (our primary value-driving screens use this feature extensively) and I'm having a hell of a time making things feel fast.
My real-world testing (and that stackblitz) really both demonstrate that rendering or deferring the ion-item-options has a massive performance impact. Deferring the rendering of options speeds up the initial render of the list by over 2x, cuts the rendering time more than in half, in my testing (even on fast computers).
I really feel strongly that this should not be discounted as a general performance optimization (or at least be made more easily possible / optional as I've specified above in the issue desc, but, until I work for Ionic I don't expect to have more influence than what I've said here ;)
The effect is really hit home when you run the stackblitz on a low-end mobile device (Even a Google Pixel XL2 at this point suffers), which according to latest reports, mid-range devices now make up most of the mobile market. Flagships with fast CPUs will drop even more, by percentage.
I know I'm preaching to the choir again, but while it's true that the primary performance hit might be the javascript, sometimes taking the low hanging fruit can make the stuff at the top feel more attainable, and on these devices, every little bit matters a lot.
Carry on! Thanks again.
This recording shows just how much impact deferring those options has. Now, performance tuning of those options might be another story, but... in the current state, they seem to matter a lot.
https://user-images.githubusercontent.com/362329/113888530-6d505c80-9790-11eb-83ff-cd4b445172e0.mov
Hey @liamdebeasi, I know you're super busy - sorry to bug you, is the <ion-item-sliding> refactor still on deck for V6? I have seen some of the gesture fixes come through. Thanks for those!