More icon interface #2688
More icon interface #2688
Conversation
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-2688/ |
Befkadu1
force-pushed
the
more-icon-interface
branch
from
1d15499 to
8669eae
Compare
Befkadu1
force-pushed
the
more-icon-interface
branch
from
8669eae to
b34746c
Compare
|
🎉 This PR is included in version 37.1.0-next.81 🎉 The release is available on: Your semantic-release bot 📦🚀 |
| {item.icon ? this.renderIcon(this.config, item) : null} | ||
| {this.renderIcon(this.config, item)} |
There was a problem hiding this comment.
😬
This change makes the list crash if any item doesn't have icon specified 😞
Reports of several customers having this problem: https://forum.lime-crm.com/t/query-visualizer-not-working-with-newer-lime-elements/5723
The problem is that renderIcon uses getIconName, which in turn does:
if (typeof icon === 'object' && 'name' in icon) {
return icon.name;
}The problem is that typeof null returns 'object', so this tries to run 'name' in null, which causes an error.
I think get-icon-props.ts is a pretty perfect example of how helper-functions can be broken out into a separate file, to make them testable. But unfortunately, there are no tests for that file 🙈 😄
I'll look into adding some tests and fixing the bug.
There was a problem hiding this comment.
omg I did not know that typeof null returns 'object' 🙈. Sorry for the bug we created! Thanks for letting us know.
|
🎉 This PR is included in version 37.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 37.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
fix #2686
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: