Fix spacing of fields in org settings: fixes (#3612) (#3681)

* add row colums styling to organisation settings fields

* add tests to make sure the grid layout is rendered

* added margin to make spacing look similar to before

* remove negative margin and reduce width

* added jest tests to make sure component mounts and that it has correct styles

* revert unnecessary changes related to cypress tests

* added strict checking for null to avoid ts errors

* use early returns on file empty

Co-authored-by: Hetu Nandu <hetunandu@gmail.com>

* use early returns on file empty

Co-authored-by: Hetu Nandu <hetunandu@gmail.com>

* fixed typo

* added findbytext for sync call

* removed unused imports

Co-authored-by: Hetu Nandu <hetunandu@gmail.com>

app/client/src/components/ads/FilePicker.tsx

@@ -194,9 +194,10 @@ const FilePickerComponent = (props: FilePickerProps) => {
   function onDrop(monitor: DropTargetMonitor) {
     if (monitor) {
       const files = monitor.getItem().files;
-      if (files) {
+      if (!files) {
-        handleFileUpload(files);
+        return;
       }
+      handleFileUpload(files);
     }
   }
 
@@ -221,10 +222,11 @@ const FilePickerComponent = (props: FilePickerProps) => {
     const file = files && files[0];
     let fileSize = 0;
 
-    if (file) {
+    if (!file) {
-      fileSize = Math.floor(file.size / 1024);
+      return;
-      setFileInfo({ name: file.name, size: fileSize });
     }
+    fileSize = Math.floor(file.size / 1024);
+    setFileInfo({ name: file.name, size: fileSize });
 
     if (fileSize < 250) {
       if (bgRef.current) {

app/client/src/pages/organization/General.tsx

@@ -24,7 +24,7 @@ import FilePicker, {
 import { getIsFetchingApplications } from "selectors/applicationSelectors";
 
 const InputLabelWrapper = styled.div`
-  width: 200px;
+  width: 150px;
   display: flex;
   align-items: center;
 `;
@@ -54,6 +54,18 @@ const FilePickerLoader = styled.div`
   border-radius: 0;
 `;
 
+// testing
+export const Row = styled.div`
+  margin: 0;
+  padding: 0;
+  display: flex;
+`;
+
+export const Col = styled.div`
+  margin: 0;
+  padding: 0;
+`;
+
 export function GeneralSettings() {
   const { orgId } = useParams<{ orgId: string }>();
   const dispatch = useDispatch();
@@ -121,75 +133,103 @@ export function GeneralSettings() {
 
   return (
     <>
-      <SettingsHeading type={TextType.H2}>General</SettingsHeading>
+      <SettingsHeading type={TextType.H2}>
+        <Row>
+          <Col>General</Col>
+        </Row>
+      </SettingsHeading>
       <SettingWrapper>
-        <InputLabelWrapper>
+        <Row>
-          <Text type={TextType.H4}>Organization Name</Text>
+          <Col>
-        </InputLabelWrapper>
+            <InputLabelWrapper>
-        {isFetchingApplications && (
+              <Text type={TextType.H4}>Organization Name</Text>
-          <Loader className={Classes.SKELETON}></Loader>
+            </InputLabelWrapper>
-        )}
+          </Col>
-        {!isFetchingApplications && (
+          <Col>
-          <TextInput
+            {isFetchingApplications && (
-            validator={notEmptyValidator}
+              <Loader className={Classes.SKELETON}></Loader>
-            placeholder="Organization Name"
+            )}
-            onChange={onWorkspaceNameChange}
+            {!isFetchingApplications && (
-            defaultValue={currentOrg && currentOrg.name}
+              <TextInput
-            cypressSelector="t--org-name-input"
+                validator={notEmptyValidator}
-          ></TextInput>
+                placeholder="Organization Name"
-        )}
+                onChange={onWorkspaceNameChange}
+                defaultValue={currentOrg && currentOrg.name}
+                cypressSelector="t--org-name-input"
+              ></TextInput>
+            )}
+          </Col>
+        </Row>
       </SettingWrapper>
 
       <SettingWrapper>
-        <InputLabelWrapper>
+        <Row className="t--organization-settings-filepicker">
-          <Text type={TextType.H4}>Upload Logo</Text>
+          <Col>
-        </InputLabelWrapper>
+            <InputLabelWrapper>
-        {isFetchingOrg && (
+              <Text type={TextType.H4}>Upload Logo</Text>
-          <FilePickerLoader className={Classes.SKELETON}></FilePickerLoader>
+            </InputLabelWrapper>
-        )}
+          </Col>
-        {!isFetchingOrg && (
+          <Col>
-          <FilePicker
+            {isFetchingOrg && (
-            url={currentOrg && currentOrg.logoUrl}
+              <FilePickerLoader className={Classes.SKELETON}></FilePickerLoader>
-            fileUploader={FileUploader}
+            )}
-            onFileRemoved={DeleteLogo}
+            {!isFetchingOrg && (
-            logoUploadError={logoUploadError.message}
+              <FilePicker
-          />
+                url={currentOrg && currentOrg.logoUrl}
-        )}
+                fileUploader={FileUploader}
+                onFileRemoved={DeleteLogo}
+                logoUploadError={logoUploadError.message}
+              />
+            )}
+          </Col>
+        </Row>
       </SettingWrapper>
 
       <SettingWrapper>
-        <InputLabelWrapper>
+        <Row>
-          <Text type={TextType.H4}>Website</Text>
+          <Col>
-        </InputLabelWrapper>
+            <InputLabelWrapper>
-        {isFetchingApplications && (
+              <Text type={TextType.H4}>Website</Text>
-          <Loader className={Classes.SKELETON}></Loader>
+            </InputLabelWrapper>
-        )}
+          </Col>
-        {!isFetchingApplications && (
+          <Col>
-          <TextInput
+            {isFetchingApplications && (
-            placeholder="Your website"
+              <Loader className={Classes.SKELETON}></Loader>
-            onChange={onWebsiteChange}
+            )}
-            defaultValue={(currentOrg && currentOrg.website) || ""}
+            {!isFetchingApplications && (
-            cypressSelector="t--org-website-input"
+              <TextInput
-          ></TextInput>
+                placeholder="Your website"
-        )}
+                onChange={onWebsiteChange}
+                defaultValue={(currentOrg && currentOrg.website) || ""}
+                cypressSelector="t--org-website-input"
+              ></TextInput>
+            )}
+          </Col>
+        </Row>
       </SettingWrapper>
 
       <SettingWrapper>
-        <InputLabelWrapper>
+        <Row>
-          <Text type={TextType.H4}>Email</Text>
+          <Col>
-        </InputLabelWrapper>
+            <InputLabelWrapper>
-        {isFetchingApplications && (
+              <Text type={TextType.H4}>Email</Text>
-          <Loader className={Classes.SKELETON}></Loader>
+            </InputLabelWrapper>
-        )}
+          </Col>
-        {!isFetchingApplications && (
+          <Col>
-          <TextInput
+            {isFetchingApplications && (
-            validator={emailValidator}
+              <Loader className={Classes.SKELETON}></Loader>
-            placeholder="Email"
+            )}
-            onChange={onEmailChange}
+            {!isFetchingApplications && (
-            defaultValue={(currentOrg && currentOrg.email) || ""}
+              <TextInput
-            cypressSelector="t--org-email-input"
+                validator={emailValidator}
-          ></TextInput>
+                placeholder="Email"
-        )}
+                onChange={onEmailChange}
+                defaultValue={(currentOrg && currentOrg.email) || ""}
+                cypressSelector="t--org-email-input"
+              ></TextInput>
+            )}
+          </Col>
+        </Row>
       </SettingWrapper>
     </>
   );

app/client/src/pages/organization/__tests__/OrganizationSettingsForm.test.tsx

@@ -0,0 +1,28 @@
+import React from "react";
+import { unmountComponentAtNode } from "react-dom";
+import { render } from "test/testUtils";
+import "@testing-library/jest-dom";
+import { GeneralSettings } from "../General";
+
+let container: any = null;
+describe("Application Settings", () => {
+  beforeEach(async () => {
+    // setup a DOM element as a render target
+    container = document.createElement("div");
+    document.body.appendChild(container);
+  });
+  it("checks that organization settings have correct styling", async (done) => {
+    const { findByText } = render(<GeneralSettings />);
+
+    const orgNameField = await findByText("Organization Name");
+    expect(orgNameField.closest("div")).toHaveStyle({ width: "150px;" });
+
+    await done();
+  });
+  afterEach(() => {
+    // cleanup on exiting
+    unmountComponentAtNode(container);
+    container.remove();
+    container = null;
+  });
+});