fix(content-uploader): resolve ItemAction issues #3631
Conversation
greg-in-a-box
changed the title
tjuanitas
force-pushed
the
fix-item-action-icons
branch
from
4b937c4 to
3868277
Compare
There was a problem hiding this comment.
this is a duplicate file, there's already an existing tsx file
| {...resin} | ||
| /> | ||
| <Tooltip content={tooltipText}> | ||
| <IconButton aria-label={tooltipText} disabled={isDisabled} onClick={onClick} icon={XMark} {...resin} /> |
There was a problem hiding this comment.
Just want to make sure that it is intended to use the default icon styles here while overriding the prop like color for XMark?
There was a problem hiding this comment.
good question. yes this is intended since IconButton has styles that will handle different states like hovering. if we override color than the styles wouldn't match the rest of the design
in the case of IconInProgress, this is a custom component that handles the loading state together with the icon. but it should probably be refactored because i don't think it's good for accessibility
| tooltip = messages.uploadsCancelButtonTooltip; | ||
| } | ||
| break; | ||
| case STATUS_PENDING: | ||
| default: | ||
| if (isResumableUploadsEnabled) { | ||
| isLoading = true; | ||
| Icon = () => ( | ||
| <LoadingIndicator aria-label={formatMessage(messages.loading)} className="bcu-ItemAction-loading" /> |
There was a problem hiding this comment.
I saw this line is duplicate with the STATUS_STAGED case above. Could we deduplicate it by having it written separately since the LoadingIndicator would kind be fixed.
const LoadingIndicatorIcon = () => <LoadingIndicator aria-label={formatMessage(messages.loading)} className="bcu-ItemAction-loading" /> And replaced the current line with Icon = LoadingIndicatorIcon;
Confirmed with designer that
Fillicons should be used for icon buttons.Before
After
Before
After
Before
After
