|
|
Added in 5.0.3 |
|
|
This fix caused problems with displaying of some icons in several places. Problems found in: Track Properties - Custom tab Track Properties - Basic tab for Youtube tracks (icons next to Location) Options - Collections & Views Options - Auto-organize Options - Media Sharing - Options... - Auto-conversion (and related Set Formats dialog) Options - Layout - Preview - Advanced - Choose fields dialog on Setup button Album view - Browser - Youtube tracks icons |
|
|
Fixed |
|
|
I still see the double icon issue occisionaly, e.g. for a playlist in the playlist header (other playlists does not exhibits the problem). It's random. image.png (4,048 bytes) |
|
|
And another regression I found is that when there is 'Grid (by Album)' view then the view icon button does not expand. Not sure how it is related, but reverting the changes to this issue does the trick, to be found why this is happening... EDIT: The change to loadIconFast in popupmenu.js is responsible for this, reverting the change fixes the issue with the menu not opened at all |
|
|
Assigned to me to resolve these regressions asap. |
|
|
I fixed various further regressions that I found Namely: 1) if svg failed to load (like in case of missing 'resize' icon) then the callback wasn't called at all (causing the view menu to not open at all) 2) fixed doubling the icon sometimes by introducing setIconFast(div, icon) that cleans the current content before setting the new icon and ensuring that the new icon exists BTW: Are you sure that loadIconFast is actually faster than loadIcon ??? From what I am seeing in the loadIcon code there is already the caching available, see var loadedData = loadedIconsHTML[iconName]; |
|
|
As discussed offline: As for speed: Yes, I've tested and it is significantly faster. The performance improvement increases exponentially with SVG size/complexity - but even for the simple icons in the Material skins, cloneNode + appendChild is much faster than setting innerHTML. I believe it's because you set HTML, the engine has to do a decent amount more work parsing the string - while if you're cloning a node, it already knows its memory structure and just has to copy it. Similar in concept to how JSON.parse() on a long string is faster than an object literal. Setting innerHTML = "" (from the recent fix) is very fast however, and completes in nanoseconds if the div was empty to begin with. (Microseconds if there is a little bit of HTML in the div). The alternative [ while(div.lastChild) div.removeChild(div.lastChild); ] is notably slower than setting innerHTML = "". There's probably a special case in the native method for clearing a div via that method. |
|
|
Verified 2619 @drakinite do you think that we should document that special case so that can be considered for developers? |
|
|
Re Setting innerHTML = "" vs. while(div.lastChild) div.removeChild(div.lastChild): No, I do not believe this is necessary to document. I don't think the wiki should include weird details about JS best practices or how to maximize performance; it should focus on how to make addons for MediaMonkey. I'm still torn on 0018145 being included in the guide. Re explanations on loadIconFast vs. loadIcon: Added to https://www.mediamonkey.com/wiki/Getting_Started_(Addons)#Icons |
- Anonymous
