From d32990ec4c0b5baad9c081e45921aba0ac93c860 Mon Sep 17 00:00:00 2001
From: Thomas Heartman <thomas@getunleash.io>
Date: Mon, 15 Jul 2024 14:41:45 +0200
Subject: [PATCH] fix: make loader not exlpode to 100vh in unnecessary
 locations (#7589)

This change fixes an issue with the loader where it would explode its
parent component to 100vh even when that was not called for.

To do so, I've added the a new `type` prop to the component, to
distinguish between `fullscreen` and `inline` usage. The `fullscreen`
type sets the height to 100vh, while the `inline` type sets it to
100%.

Now, this doesn't directly make the loader fullscreen (it just makes
sure it's at least as tall as the screen), so maybe the prop name is
misleading. I'd be happy to change it (or to even extract this into
two separate components) if that's preferable. Other potential prop
names could be `height`, which is very direct, or `usage`, which I
think better describes what we do. Like with `type`, I'd like to
communicate the intended behavior more that the actual implementation,
so I'm leaning towards either `type` or `usage`.

## Screenies

I've gone through all the usages of the loader, and checked how each one
works. Here they are:

### Loader in environment variants

I wasn't able to trigger this manually, but it's apparently there

Old (ignore the banner placement; that's firefox's screenshot tool
acting up)

![image](https://github.com/user-attachments/assets/f5d0a709-6815-4838-9ad4-c8f79a54ad0e)

New:

![image](https://github.com/user-attachments/assets/c7538146-b8af-4253-89ed-55d1eb37d6a5)

### Project setting forms

Old:

![image](https://github.com/user-attachments/assets/f8b55899-4483-470a-8d3a-3d11761ec8c7)

New:

![image](https://github.com/user-attachments/assets/29157004-6662-494b-9939-2f34977a9c63)

### Rollout strategy

Old:

![image](https://github.com/user-attachments/assets/5c699a06-37bd-4b3b-a3e3-f613ca7c88d5)

New (no discernible change):

![image](https://github.com/user-attachments/assets/f52178fe-9d26-4ebb-bd48-8a1c4a7e2f04)

### Advanced playground

Old:

![image](https://github.com/user-attachments/assets/43f7183b-cefc-4e29-961e-5d7e18d29be9)

New:

![image](https://github.com/user-attachments/assets/082d94dc-36e0-483c-b7a9-bd75e727c0d5)

### Loading screen / initial redirect

Old:

![image](https://github.com/user-attachments/assets/dbb8b1af-d585-4d48-8431-5379afd4f653)

New (no new component props):

![image](https://github.com/user-attachments/assets/842e766f-0ea7-4396-9696-b88509e24d77)

New (with new props):

![image](https://github.com/user-attachments/assets/e6ffd303-f24e-478d-88d9-b4fa57f307e4)
---
 frontend/src/component/App.tsx                  |  4 ++--
 frontend/src/component/common/Loader/Loader.tsx | 15 +++++++++++----
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/frontend/src/component/App.tsx b/frontend/src/component/App.tsx
index 946f3728d1..f71425e5f4 100644
--- a/frontend/src/component/App.tsx
+++ b/frontend/src/component/App.tsx
@@ -50,10 +50,10 @@ export const App = () => {
 
     return (
         <SWRProvider>
-            <Suspense fallback={<Loader />}>
+            <Suspense fallback={<Loader type='fullscreen' />}>
                 <ConditionallyRender
                     condition={!hasFetchedAuth}
-                    show={<Loader />}
+                    show={<Loader type='fullscreen' />}
                     elseShow={
                         <Demo>
                             <>
diff --git a/frontend/src/component/common/Loader/Loader.tsx b/frontend/src/component/common/Loader/Loader.tsx
index 2bbbce2ec9..9cf537f484 100644
--- a/frontend/src/component/common/Loader/Loader.tsx
+++ b/frontend/src/component/common/Loader/Loader.tsx
@@ -1,12 +1,19 @@
 import logo from 'assets/img/unleashLogoIconDarkAlpha.gif';
 import { formatAssetPath } from 'utils/formatPath';
 import { styled } from '@mui/material';
+import type { FC } from 'react';
 
-const StyledDiv = styled('div')(({ theme }) => ({
+type LoaderProps = {
+    type?: 'fullscreen' | 'inline';
+};
+
+const StyledDiv = styled('div', {
+    shouldForwardProp: (prop) => prop !== 'type',
+})<{ type: LoaderProps['type'] }>(({ theme, type }) => ({
     display: 'flex',
     justifyContent: 'center',
     alignItems: 'center',
-    minHeight: '100vh',
+    height: type === 'fullscreen' ? '100vh' : '100%',
     backgroundColor: theme.palette.background.paper,
 }));
 
@@ -15,9 +22,9 @@ const StyledImg = styled('img')(({ theme }) => ({
     height: '100px',
 }));
 
-const Loader = () => {
+const Loader: FC<LoaderProps> = ({ type = 'inline' }) => {
     return (
-        <StyledDiv role='alert' aria-label='Loading'>
+        <StyledDiv role='alert' aria-label='Loading' type={type}>
             <StyledImg src={formatAssetPath(logo)} alt='' />
         </StyledDiv>
     );