Skip to content

Commit ca1dff1

Browse files
krazziekaykrazziekay
authored
5KU-ENHANCEMENT - Customized scrollbars added (#2457)
* - Customized scrollbars added - Gradient styling removed * - Fixing test case * - Fixing test case * - Review changes
1 parent 8d6e540 commit ca1dff1

File tree

6 files changed

+192
-131
lines changed

6 files changed

+192
-131
lines changed

‎spa/package.json‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,8 @@
3636
"axios": "^1.4.0",
3737
"react": "^18.2.0",
3838
"react-dom": "^18.2.0",
39-
"react-router-dom": "^6.14.1"
39+
"react-router-dom": "^6.14.1",
40+
"simplebar-react": "^3.2.4"
4041
},
4142
"devDependencies": {
4243
"@testing-library/jest-dom": "^5.16.5",

‎spa/src/common/Scrollbars.tsx‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/** @jsxImportSource @emotion/react */
2+
import SimpleBar from "simplebar-react";
3+
import "simplebar-react/dist/simplebar.min.css";
4+
5+
// Source: https://github.com/Grsmto/simplebar/blob/master/packages/simplebar/README.md#options
6+
const Scrollbars = ({
7+
style,
8+
children
9+
}: {
10+
style: React.CSSProperties | undefined,
11+
children: React.JSX.Element
12+
}) =>
13+
<SimpleBar
14+
forceVisible="y"
15+
autoHide={false}
16+
style={style}
17+
>
18+
{children}
19+
</SimpleBar>;
20+
21+
export default Scrollbars;

‎spa/src/helper/useOrgListScroll.tsx‎

Lines changed: 0 additions & 48 deletions
This file was deleted.

‎spa/src/pages/ConfigSteps/OrgsContainer/index.tsx‎

Lines changed: 65 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -11,25 +11,22 @@ import {
1111
ErrorForNonAdmins,
1212
ErrorForSSO,
1313
} from "../../../components/Error/KnownErrors";
14-
import useOrgListScroll from "../../../helper/useOrgListScroll";
14+
import Scrollbars from "../../../common/Scrollbars";
15+
16+
const MAX_HEIGHT_FOR_ORGS_CONTAINER = 250;
17+
const PADDING_RIGHT_FOR_ORGS_CONTAINER = 80;
18+
const MARGIN_RIGHT_FOR_ORGS_CONTAINER = -80;
1519

16-
const orgsWrapperStyle = css`
17-
max-height: 250px;
18-
overflow-y: auto;
19-
width: 100%;
20-
`;
2120
const orgDivStyle = css`
2221
display: flex;
2322
justify-content: space-between;
2423
align-items: center;
2524
padding: ${token("space.150")} 0;
2625
margin-bottom: ${token("space.100")};
2726
`;
28-
2927
const orgDivWithErrorStyle = css`
3028
align-items: start;
3129
`;
32-
3330
const orgNameStyle = css`
3431
color: ${token("color.text")};
3532
font-weight: 590;
@@ -38,15 +35,6 @@ const iconWrapperStyle = css`
3835
padding-top: ${token("space.150")};
3936
`;
4037

41-
const gradientStyle = css`
42-
background: linear-gradient(rgba(255, 255, 255, 0), rgb(255, 255, 255));
43-
height: 70px;
44-
margin-top: -70px;
45-
position: relative;
46-
width: 100%;
47-
display: block;
48-
`;
49-
5038
const OrganizationsList = ({
5139
organizations,
5240
loaderForOrgClicked,
@@ -61,12 +49,6 @@ const OrganizationsList = ({
6149
resetCallback: (args: boolean) => void;
6250
connectingOrg: (org: GitHubInstallationType) => void;
6351
}) => {
64-
const {
65-
isScrolledToBottom,
66-
isListScrollable,
67-
containerRef,
68-
hasUserScrolledRef,
69-
} = useOrgListScroll();
7052
const [clickedOrg, setClickedOrg] = useState<
7153
GitHubInstallationType | undefined
7254
>(undefined);
@@ -103,66 +85,68 @@ const OrganizationsList = ({
10385
}
10486
};
10587
return (
106-
<>
107-
<div css={orgsWrapperStyle} ref={containerRef}>
88+
<Scrollbars
89+
style={{
90+
maxHeight: MAX_HEIGHT_FOR_ORGS_CONTAINER,
91+
paddingRight: PADDING_RIGHT_FOR_ORGS_CONTAINER,
92+
marginRight: MARGIN_RIGHT_FOR_ORGS_CONTAINER
93+
}}
94+
>
95+
<>
10896
{organizations.map((org) => {
109-
const hasError = !canConnect(org);
110-
const orgDivStyles = hasError
111-
? [orgDivStyle, orgDivWithErrorStyle]
112-
: [orgDivStyle];
113-
return (
114-
<div key={org.id} css={orgDivStyles}>
115-
{canConnect(org) ? (
116-
<>
117-
<span css={orgNameStyle}>{org.account.login}</span>
118-
{loaderForOrgClicked && clickedOrg?.id === org.id ? (
119-
<LoadingButton style={{ width: 80 }} isLoading>
120-
Loading button
121-
</LoadingButton>
122-
) : (
123-
<Button
124-
isDisabled={
125-
loaderForOrgClicked && clickedOrg?.id !== org.id
126-
}
127-
onClick={async () => {
128-
setLoaderForOrgClicked(true);
129-
setClickedOrg(org);
130-
try {
131-
// Calling the create connection function that is passed from the parent
132-
await connectingOrg(org);
133-
} finally {
134-
setLoaderForOrgClicked(false);
135-
}
136-
}}
137-
>
138-
Connect
139-
</Button>
140-
)}
141-
</>
142-
) : (
143-
<>
144-
<div>
97+
const hasError = !canConnect(org);
98+
const orgDivStyles = hasError
99+
? [orgDivStyle, orgDivWithErrorStyle]
100+
: [orgDivStyle];
101+
return (
102+
<div key={org.id} css={orgDivStyles}>
103+
{canConnect(org) ? (
104+
<>
145105
<span css={orgNameStyle}>{org.account.login}</span>
146-
<div>{errorMessage(org)}</div>
147-
</div>
148-
<div css={iconWrapperStyle}>
149-
<WarningIcon
150-
label="warning"
151-
primaryColor={token("color.background.warning.bold")}
152-
size="medium"
153-
/>
154-
</div>
155-
</>
156-
)}
157-
</div>
158-
);
159-
})}
160-
</div>
161-
{isListScrollable &&
162-
(hasUserScrolledRef.current ? !isScrolledToBottom : true) && (
163-
<div css={gradientStyle} />
164-
)}
165-
</>
106+
{loaderForOrgClicked && clickedOrg?.id === org.id ? (
107+
<LoadingButton style={{ width: 80 }} isLoading>
108+
Loading button
109+
</LoadingButton>
110+
) : (
111+
<Button
112+
isDisabled={
113+
loaderForOrgClicked && clickedOrg?.id !== org.id
114+
}
115+
onClick={async () => {
116+
setLoaderForOrgClicked(true);
117+
setClickedOrg(org);
118+
try {
119+
// Calling the create connection function that is passed from the parent
120+
await connectingOrg(org);
121+
} finally {
122+
setLoaderForOrgClicked(false);
123+
}
124+
}}
125+
>
126+
Connect
127+
</Button>
128+
)}
129+
</>
130+
) : (
131+
<>
132+
<div>
133+
<span css={orgNameStyle}>{org.account.login}</span>
134+
<div>{errorMessage(org)}</div>
135+
</div>
136+
<div css={iconWrapperStyle}>
137+
<WarningIcon
138+
label="warning"
139+
primaryColor={token("color.background.warning.bold")}
140+
size="medium"
141+
/>
142+
</div>
143+
</>
144+
)}
145+
</div>
146+
);
147+
})}
148+
</>
149+
</Scrollbars>
166150
);
167151
};
168152

‎spa/src/pages/ConfigSteps/test.tsx‎

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,14 @@ jest.mock("react-router-dom", () => ({
1313
useNavigate: () => jest.fn(),
1414
}));
1515

16+
// Mock the import of the css
17+
jest.mock("simplebar-react/dist/simplebar.min.css", () => "");
18+
// Mock the simplebar-react component
19+
jest.mock("simplebar-react", () => ({
20+
__esModule: true,
21+
default: ({children}: {children: React.JSX.Element}) => <div>{children}</div>,
22+
}));
23+
1624
/* eslint-disable react-refresh/only-export-components */
1725
const Authenticated = {
1826
checkValidity: jest.fn().mockReturnValue(Promise.resolve(true)),
@@ -24,6 +32,16 @@ const Authenticated = {
2432
getUserDetails: jest.fn().mockReturnValue({ username: "kay", email: "kay"}),
2533
clear: jest.fn(),
2634
};
35+
const AuthenticatedWithOrgsWithErrors = {
36+
checkValidity: jest.fn().mockReturnValue(Promise.resolve(true)),
37+
authenticateInGitHub: jest.fn().mockReturnValue(Promise),
38+
fetchOrgs: jest.fn().mockReturnValue({ orgs: [ { account: { login: "org-1" }, id: 1, isAdmin: false }, { account: { login: "org-2" }, id: 2, requiresSsoLogin: true }, { account: { login: "org-3" }, id: 3, isIPBlocked: true }, { account: { login: "org-4" }, id: 4, isAdmin: true } ]}),
39+
installNewApp: jest.fn(),
40+
connectOrg: jest.fn(),
41+
setTokens: jest.fn(),
42+
getUserDetails: jest.fn().mockReturnValue({ username: "kay", email: "kay"}),
43+
clear: jest.fn(),
44+
};
2745
const AuthenticatedWithNoOrgs = {
2846
checkValidity: jest.fn().mockReturnValue(Promise.resolve(true)),
2947
authenticateInGitHub: jest.fn().mockReturnValue(Promise),
@@ -33,7 +51,6 @@ const AuthenticatedWithNoOrgs = {
3351
getUserDetails: jest.fn().mockReturnValue({ username: "kay", email: "kay"}),
3452
clear: jest.fn(),
3553
};
36-
3754
/* eslint-disable react-refresh/only-export-components */
3855
const UnAuthenticated = {
3956
checkValidity: jest.fn().mockReturnValue(Promise.resolve(false)),
@@ -166,6 +183,52 @@ test("Connect GitHub Screen - Checking the GitHub Cloud flow when authenticated
166183
expect(AppManager.connectOrg).toBeCalled();
167184
});
168185

186+
test("Connect GitHub Screen - Checking the GitHub Cloud flow when authenticated with orgs with errors", async () => {
187+
jest.mocked(OAuthManager).getUserDetails = AuthenticatedWithOrgsWithErrors.getUserDetails;
188+
jest.mocked(OAuthManager).checkValidity = AuthenticatedWithOrgsWithErrors.checkValidity;
189+
jest.mocked(AppManager).fetchOrgs = AuthenticatedWithOrgsWithErrors.fetchOrgs;
190+
jest.mocked(AppManager).installNewApp = AuthenticatedWithOrgsWithErrors.installNewApp;
191+
jest.mocked(AppManager).connectOrg = jest.fn().mockImplementation(async () => {
192+
//do not return, so that to assert on the loading icon
193+
return new Promise(_ => {});
194+
});
195+
196+
const { container } = render(
197+
<BrowserRouter>
198+
<ConfigSteps />
199+
</BrowserRouter>
200+
);
201+
await act(async () => container);
202+
203+
expect(screen.queryByText(GITHUB_CLOUD)).not.toBeInTheDocument();
204+
expect(screen.queryByText(GITHUB_ENTERPRISE)).not.toBeInTheDocument();
205+
expect(screen.queryByText("Next ")).not.toBeInTheDocument();
206+
expect(screen.queryByText(SELECT_GH_PRODUCT)).not.toBeInTheDocument();
207+
208+
// Checking if all the orgs are being displayed
209+
expect(screen.queryByText(SELECT_GH_TEXT)).toBeInTheDocument();
210+
expect(screen.queryByText("org-1")).toBeInTheDocument();
211+
expect(screen.queryByText("org-2")).toBeInTheDocument();
212+
expect(screen.queryByText("org-3")).toBeInTheDocument();
213+
expect(screen.queryByText("org-4")).toBeInTheDocument();
214+
215+
// 3 orgs have errors, only 1 can be connected
216+
expect(await screen.findAllByRole("button", { name: "Connect" })).toHaveLength(1);
217+
218+
const errorForNonAdmins = container.querySelectorAll("[class$='-ErrorForNonAdmins']");
219+
expect(errorForNonAdmins[0].textContent).toBe("Can't connect, you're not the organization owner.Ask an organization owner to complete this step.");
220+
221+
const errorForSSO = container.querySelectorAll("[class$='-ErrorForSSO']");
222+
expect(errorForSSO[0].textContent).toBe("Can't connect, single sign-on(SSO) required.");
223+
expect(errorForSSO[1].textContent).toBe("1. Log into GitHub with SSO.");
224+
expect(errorForSSO[3].textContent).toBe("2. Retry connection in Jira (once logged in).");
225+
226+
const errorForIPBlocked = container.querySelectorAll("[class$='-ErrorForIPBlocked']");
227+
expect(errorForIPBlocked[0].textContent).toBe("Can't connect, blocked by your IP allow list.");
228+
expect(errorForIPBlocked[1].textContent).toBe("How to update allowlist");
229+
expect(errorForIPBlocked[3].textContent).toBe("Retry");
230+
});
231+
169232
test("Connect GitHub Screen - Checking the GitHub Cloud flow when authenticated with no orgs", async () => {
170233
jest.mocked(OAuthManager).getUserDetails = AuthenticatedWithNoOrgs.getUserDetails;
171234
jest.mocked(OAuthManager).checkValidity = AuthenticatedWithNoOrgs.checkValidity;
@@ -220,3 +283,4 @@ test("Connect GitHub Screen - Changing GitHub login when authenticated", async (
220283
expect(screen.queryByText(SELECT_GH_PRODUCT_CTA)).toBeInTheDocument();
221284
});
222285

286+

0 commit comments

Comments
 (0)