(feat) O3-4249: Add component to support rendering an icon if it exists #1220
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ibacher
requested review from
denniskigen,
samuelmale,
jayasanka-sack,
vasharma05 and
NethmiRodrigo
|
Size Change: -148 kB (-2.35%) Total Size: 6.15 MB
ℹ️ View Unchanged
|
vasharma05
approved these changes
Dec 3, 2024
There was a problem hiding this comment.
Thank you @ibacher for working on this. I was about to discuss about the same in a few days, so happy to see this!
ibacher
commented
Dec 3, 2024
| } | ||
| }; | ||
|
|
||
| const observer = new MutationObserver(callback); |
There was a problem hiding this comment.
Note to self: I'm not sure about the performance implications of having a per-icon mutation observer vs, say, a global mutation observer that could have callbacks registered with it. This is, however, easier to write.
denniskigen
approved these changes
Dec 4, 2024
There was a problem hiding this comment.
Thanks, @ibacher. LGTM. I've left a few persnickety comments.
packages/framework/esm-react-utils/src/RenderIfValueIsTruthy.tsx
Outdated
Show resolved
Hide resolved
packages/framework/esm-styleguide/src/pictograms/pictograms.tsx
Outdated
Show resolved
Hide resolved
packages/framework/esm-styleguide/src/pictograms/pictograms.tsx
Outdated
Show resolved
Hide resolved
| * <MaybeIcon icon='omrs-icon-baby' className={styles.myIconStyles} /> | ||
| * ``` | ||
| */ | ||
| export const MaybeIcon = memo( |
There was a problem hiding this comment.
Suggested change
| export const MaybeIcon = memo( | |
| export const MaybeIcon = memo( |
nit: Maybe LoadableIcon reads better here?
| } | ||
| }; | ||
|
|
||
| const observer = new MutationObserver(callback); |
NethmiRodrigo
approved these changes
Dec 4, 2024
Co-authored-by: Dennis Kigen <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Requirements
feat,fix, orchore, among others). See existing PR titles for inspiration.For changes to apps
If applicable
Summary
This is pretty straight-forward. It adds a couple of utility components:
RenderIfValueIsTruthy— Does what it says; either renders the provided children if thevalueprop is truthy or else renders a specified fallback (default nothing). This is just a building block for the functionality added here, but it seemed like it may be more broadly useful.MaybeIcon— This is basically a combination ofRenderIfValueIsTruthyand the existingIconcomponent, so that it renders as an icon if the icon specified as a string is one of the icons provided and as nothing if the icon isn't defined.MaybePictogram— Same thing, but wrapping pictograms.The intended use case is to support configuration-driven icons, for example:
As long as
config.iconresolves to a known icon id, e.g.,arrow-up, this will render exactly like theArrowUpIconcomponent, but ifconfig.iconspecifies something that doesn't exist, nothing will be rendered at all.Screenshots
Related Issue
Other
This also adds some docs for
UserHasAccesswhich is kind of similar toRenderIfValueIsTruthy.