From c5a7737790857b295f9656404843bf472dbf3870 Mon Sep 17 00:00:00 2001 From: telamonian Date: Tue, 14 Jan 2020 05:41:52 -0500 Subject: [PATCH 1/3] added notes about JLIcon's design to README --- packages/ui-components/README.md | 44 ++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/packages/ui-components/README.md b/packages/ui-components/README.md index 9769e909b04a..f6aca71864d3 100644 --- a/packages/ui-components/README.md +++ b/packages/ui-components/README.md @@ -43,3 +43,47 @@ The icons are organized into various categories in `./style/icons`, based on whe - launcher - statusbar - settingeditor + +## Notes on design of JLIcon + +Design goals for JLIcon (already implemented): + +- the one true icon + - create a single, canonical, simple (as possible) way to set up and use icons in jlab +- every icon is a symbol + - each icon is defined exactly once, and is thereafter referred to via a unique symbol + - to use an icon `fooIcon` outside of the file in which it is defined, the pattern will be to import `fooIcon` + - this enables compile-time checking; helps to ensure that icons are specified correctly when used (as opposed to the old pattern of specifying icon via a `string` with name or className) +- every icon is flexible + - can used in any context in which icons are used in jlab + - every icon can be passsed into Lumino + - every icon can create a DOM node + - every icon can create a React component +- dyanmic lookup (for when absolutely needed) + - Use dynamic lookup for the few cases in which an icon can only be specified as a string (such as in json schema files) + - In all other cases, force (or at least strongly encourage) the pattern of import-icon +- reusable + - every defined icon can be used any number of times in any set of contexts + - where an icon is defined should not matter; all icons defined in core and extensions are reusable in any other extension + +Design goals for JLIcon (partially implemented): + +- remove all other ways of creating icons (though leave an escape hatch) + - need to deprecate, then later remove, `iconClass` from a number of interfaces +- replacable + - all icons can be customized by replacing their svg dynamically during runtime + - currently, I'm leaning towards the idea that icon replacements should be an (optional) part of a theme + - ideally, replacability will not depend on dynamic lookup + - whenever its svg is replaced, all visible copies of an icon should immediately rerender + - possible implementations: + - signals + - observables + - `MutationObserver` + +Possible design patterns for JLIcon: + +1. each icon is a `class`. The icon is used by creating a new instance +2. each icon is a function with methods (ie a callable instance). The icon is used by calling the appropriate method +3. each icon is an instance of a well-defined `class`. The icon is used by calling the appropriate instance method + +Patterns 1) and 2) were both initially investigated (see [jupyterlab/jupyterlab#7299](https://github.com/jupyterlab/jupyterlab/pull/7299)). Pattern 3) was found to be easiest to reason about, and had a desirable set of tradeoffs relating to features like dynamic lookup and replacability (eg you can replace the svg of an icon by just setting the `svgstr` field of the icon instance). From fcd16f5b81bec92e8cb4c88777c5933b81ae50c2 Mon Sep 17 00:00:00 2001 From: Max Klein Date: Tue, 14 Jan 2020 05:47:04 -0500 Subject: [PATCH 2/3] fixed typo --- packages/ui-components/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui-components/README.md b/packages/ui-components/README.md index f6aca71864d3..d4b632e80403 100644 --- a/packages/ui-components/README.md +++ b/packages/ui-components/README.md @@ -59,7 +59,7 @@ Design goals for JLIcon (already implemented): - every icon can be passsed into Lumino - every icon can create a DOM node - every icon can create a React component -- dyanmic lookup (for when absolutely needed) +- dynamic lookup (for when absolutely needed) - Use dynamic lookup for the few cases in which an icon can only be specified as a string (such as in json schema files) - In all other cases, force (or at least strongly encourage) the pattern of import-icon - reusable From a297d5df59d71893d4ab841b64ad3ea39d6aa29e Mon Sep 17 00:00:00 2001 From: Max Klein Date: Tue, 14 Jan 2020 14:57:26 -0500 Subject: [PATCH 3/3] fixed typo Co-Authored-By: Jeremy Tuloup --- packages/ui-components/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui-components/README.md b/packages/ui-components/README.md index d4b632e80403..71c67481081a 100644 --- a/packages/ui-components/README.md +++ b/packages/ui-components/README.md @@ -56,7 +56,7 @@ Design goals for JLIcon (already implemented): - this enables compile-time checking; helps to ensure that icons are specified correctly when used (as opposed to the old pattern of specifying icon via a `string` with name or className) - every icon is flexible - can used in any context in which icons are used in jlab - - every icon can be passsed into Lumino + - every icon can be passed into Lumino - every icon can create a DOM node - every icon can create a React component - dynamic lookup (for when absolutely needed)