(fix) 03-2588: Tweak workspace alert badge UI #1499
Conversation
jwnasambu
force-pushed
the
(fix)O3-2588
branch
from
04925e6 to
08e409c
Compare
|
@denniskigen Kindly feel free to have a look at my PR at your convinient time please! |
(fix)03-2588: Adjust the shape
denniskigen
force-pushed
the
(fix)O3-2588
branch
from
08e409c to
3a2f4fa
Compare
denniskigen
changed the title
denniskigen
changed the title
| kind="ghost" | ||
| label={label} | ||
| onClick={handler} | ||
| > | ||
| <div className={styles.elementContainer}> | ||
| {getIcon({ size: 20 })} | ||
| <span className={styles.countTag}>{formOpenInTheBackground ? '!' : tagContent}</span> | ||
| <Tags isTablet={false} /> |
There was a problem hiding this comment.
Hi @jwnasambu, as we discussed before as well, we cannot have a component inside a component. If the Tags is defined as a function, it is better to call it as {Tags()} rather than <Tags />
Please refer: https://stackoverflow.com/questions/62047511/in-react-can-i-define-a-functional-component-within-the-body-of-another-functio
CC: @denniskigen @ibacher
There was a problem hiding this comment.
@vasharma05 There' s no real superiority in calling {Tags()} rather than <Tags />. The accepted answer on that SO post is... not wrong, but terrible advice (and, importantly, in that one case, the component being rendered like that always produces the same mark-up; that's the whole reason it works) .
The right way to approach this is to extract Tags into it's own component (it can be defined in the same file) and pass the data it needs into the component as props. Then we're just using React as designed.
Requirements
Summary
I added a new class styles.interruptedTag that is conditional rendered. If formOpenInTheBackground is true, it renders a element with the class styles.interruptedTag containing an exclamation mark (With the shape of the icon being circular). If formOpenInTheBackground is false, it renders null, meaning nothing is displayed.
Screenshots
Appearance on desktop
Appearance on tablet
Related Issue
Other