[risk=low][RW-9006] Extract a runtime-status-icon component #7118
Conversation
| hasContent: boolean; | ||
| } | ||
|
|
||
| export const proIcons = { |
There was a problem hiding this comment.
IMO this indirection was more confusing than helpful
| workspaceNamespace: string, | ||
| store: RuntimeStore | ||
| ) => { | ||
| let status = store?.runtime?.status; |
There was a problem hiding this comment.
this logic is now inside the new RuntimeStatusIcon
| import { getCdrVersion } from 'app/utils/cdr-versions'; | ||
| import { ComputeSecuritySuspendedError } from 'app/utils/runtime-utils'; | ||
| import { | ||
| CompoundRuntimeOpStore, |
There was a problem hiding this comment.
only used by the runtime status icon
| icon: IconConfig, | ||
| workspace: WorkspaceData, | ||
| store: RuntimeStore, | ||
| compoundRuntimeOps: CompoundRuntimeOpStore |
There was a problem hiding this comment.
the RuntimeStatusIcon can access this store directly - no need to pass it in
| <img | ||
| data-test-id={'help-sidebar-icon-' + icon.id} | ||
| src={proIcons[icon.id]} | ||
| src={thunderstorm} |
There was a problem hiding this comment.
it's always runtime here, so this is equivalent
| <img | ||
| data-test-id={'help-sidebar-icon-' + icon.id} | ||
| src={proIcons[icon.id]} | ||
| src={icon.id === 'runtime' && thunderstorm} |
There was a problem hiding this comment.
equivalent logic because there's no other proIcons entry which matched a SidebarIconId
| } from 'generated/fetch'; | ||
|
|
||
| import { SelectionList } from 'app/cohort-search/selection-list/selection-list.component'; | ||
| import { Clickable, StyledExternalLink } from 'app/components/buttons'; |
There was a problem hiding this comment.
re-sorted after linting
| <h3 | ||
| style={{ | ||
| ...styles.sectionTitle, | ||
| marginTop: 0, |
There was a problem hiding this comment.
all uses of sectionTitle had this override, so it's now included
c490920 to
f524c23
Compare
| }, | ||
| sectionTitle: { | ||
| marginTop: '0.5rem', | ||
| marginTop: 0, |
There was a problem hiding this comment.
all uses of sectionTitle overrode marginTop with 0, so it's now included here
| asyncOperationStatusIcon: { | ||
| width: '.5rem', | ||
| height: '.5rem', | ||
| zIndex: 2, |
There was a problem hiding this comment.
Most uses of zIndex need a comment as to why one needs to override the browser's default layout model. Maybe the usage will make this obvious.
There was a problem hiding this comment.
Okay, it's not obvious to me why it is needed.
There was a problem hiding this comment.
I don't know much about how zIndex works (and this retains the existing style's value) but perhaps this is how the circle icon gets superimposed on the thunderstorm icon?
We'll need this for the new apps panel. Splitting into its own PR for ease of review.
PR checklist