etr2460 commented on a change in pull request #13179:
URL: https://github.com/apache/superset/pull/13179#discussion_r577813467
##########
File path: superset-frontend/src/components/Loading/index.tsx
##########
@@ -41,12 +42,16 @@ const LoaderImg = styled.img`
transform: translate(-50%, -50%);
}
`;
-export default function Loading({ position = 'floating' }: Props) {
+export default function Loading({
+ position = 'floating',
+ image = '/static/assets/images/loading.gif',
+}: Props) {
return (
<LoaderImg
className={`loading ${position}`}
alt="Loading..."
- src="/static/assets/images/loading.gif"
+ src={image}
+ data-test-id="loading"
Review comment:
perhaps instead of setting a test id here, we should add the appropriate
aria labels, both for a11y and so that RTL can see them.
I think `aria-busy=true` and `aria-label=loading` would be pretty standard,
and then you should be able to select it with RTL either using the new aria
labels, or with `getByAltText`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
