Add pagination to icon picker

[bjarnef]

Currently the icon picker is one long list of icons, but when having many icons in can be slow. I have previous noticed this when installing font packages, which have a lot of icons, e.g. some of these icon packages:

https://our.umbraco.com/packages/backoffice-extensions/material-design-icon-pack/
https://our.umbraco.com/packages/backoffice-extensions/fontawesome-icon-pack/
https://our.umbraco.com/packages/backoffice-extensions/line-awesome-icon-pack/

With the new feature to include SVG icons, I wonder if we should add pagination here, e.g. 100 per page.

I found this with some free SVG icons:
https://cssauthor.com/free-svg-icons/

One of the mentioned is https://iconhub.io

Performance using angular ngRepeat directive can be optimized with track by but in my experience when having e.g. 500-1000+ repeated entries in DOM, it is slowing down depending on the browser.

[bjarnef]

@MMasey @nielslyngsoe have you noticed issues with icon picker when having e.g. 1000 or 2000 icons? At the moment I think we only have a single method in C# fetching all icons or a specific icon, but not a paged list of icons?

[bjarnef]

If grabbing all icons from here https://remixicon.com and placing these inside /Umbraco/assets/icons in Umbraco v8.9.0 (or in / App_Plugins/MyPlugin/Icons folder in v8/contrib branch, and doing a search in icon picker, the search is being slow and sometimes I get this, because it use too much memory (I might be easier to reproduce when the developers tools is open).

In this case with these icons + core icons there are 2743 icons, which it seems to fetch once and the filter the results in ngRepeat. This works well for a smaller among of items, but not for 1000+ entries.

I think we should have a GetPagedIcons method e.g. something like this:

/// <summary>
/// Gets a paged list of svg icons found at at the global icons path.
/// </summary>
/// <returns></returns>
public IList<IconModel> GetPagedIcons(int pageNumber, int pageSize, string filter = "")
{
      return _iconService.GetPagedIcons(pageNumber, pageSize, filter);
}

Maybe it would also be useful to filter by group if the icons sometimes later supports this: #8884 (comment)

Something similar to GetPagedChildren in EntityController:
https://github.com/umbraco/Umbraco-CMS/blob/v8/contrib/src/Umbraco.Web/Editors/EntityController.cs#L510-L569

[MMasey]

Hey @bjarnef & @nul800sebastiaan, I've been having a think about this and I think we need to solve a number of other performance problems first, before we consider pagination. I do think some categorisation would be cool though.

I think the memory issues are primarily affecting the frontend due to the following reasons.

• When using the filtering, there doesn't seem to be an deferring so each time a character changes there is a paint reflow, a really big one too
• I think the reason the rendering takes so long is because I when the ng-repeat does the filter it added each of the icons individually. This isn't much of an issue when there are only a few icons returned by the filter, but if there are loads it completely clogs up the thread.
• Each of the SVG's themselves are also lazy loaded. This adds then another reflow when they get loaded in, so I wonder if maybe for the full icon listing we don't lazy load?

I'm thinking maybe the icon list needs a fair bit of refactoring, maybe even moving away from using the inline ng-repeat. I think a better solution in terms of updated the DOM in a perfomant way would be to do the following.

1. Remove the use of ng-repeat and the filtering from the view
2. Update the logic for the filtering to happen purely in the JavaScript. Looking at a snapshot of the filtering there is some jQuery being used internally by angular (I think) which it taking a while.
3. Add a little bit of deferring to the search field
4. Add an option to the umb-icon directive to make lazy loading optional, and set it accordingly for the icon picker
5. Rather than updating the list of icons on at a time (which is what it looks like the ng-repeat is doing, we instead wait for the list of icons to be filtered, and then add them all to a documentFragment which is then added to the DOM.

This would potentially make it a little easier moving forward as we look at detaching from angular too.

In terms of the backend performance there are a few area that could be improved. I think in a previous issue I mentioned about the use of ToList causing some perf issues, but even then it seems like it's on the clientside that it is most noticed.

Please let me know if any of that sounds wrong as i'm not 100% sure on all of it.

Thanks

[bjarnef]

@MMasey yes, the search filter should definitely has some timeout/debounce before filtering either in via ngModelOptions https://docs.angularjs.org/api/ng/directive/ngModelOptions or some delay in controller before filtering the result or requesting a filtered result server side.

In my experience ngRepeat is slowing down, when having ~500-1000+ items even without doing filtering. The track by can however improve the performance a bit.

It would be great to load more icons as you scroll down, however I think it would require some kind of paged result, but maybe it is possible to append e.g 100 icons at the times although all icons at the moment is loaded.

In this PR https://github.com/umbraco/Umbraco-CMS/pull/9359/files is it doing some Linq queries, but in my test the server side request to get 100 icons were much faster than the current request to get all icons, where I recall it took 1.5+ seconds.

Most of the time when opening the icon picker, you probably only need to see a few icons and get more as you scroll or filtering the icons.

[MMasey]

Yeah that all makes sense. I think it should be relatively easy to implement a "paged" approach as you scroll down the page for when all of the icons are present. The complicated part would be when a filter is being used, although we could duplicate the way angular filters for the backend code. Then again, we may end up having some odd issues in regards to concatenating the paginated icons to the collection of icons loaded already.

I does feel to me like it's more of a frontend issue then a backend, even if the initial loading of all the icons takes a while. That's not to say improvements can't be made on both sides, I think it's just figuring out which part to attack first. Also it looks like track by approach is already being used.

This is certainly a fun problem 😜

[elit0451]

Hey all, sorry for taking some time to return to this issue 🙈 Reading through, I will convert this to a discussion in case the mentioned functionality is still desired. 🙂