Skip to content

Commit

Permalink
fix: admin icon in sidenav (#138)
Browse files Browse the repository at this point in the history
## Description

<!-- Write and explain of the changes introduced by this PR for the reviewers to fully understand -->

## Screenshot

<!-- Provide a screenshot or gif of the change to demonstrate it -->

## Test Plan

<!-- Explain what you tested and why -->

<!--
  Have any questions? Check out the contributing doc for more
-->


---

<details open><summary>Generated summary (powered by <a href="https://app.graphite.dev">Graphite</a>)</summary>

> ## TL;DR
> This pull request updates the Axios request configuration type, and changes the way the admin icon is rendered in the GlobalNav component.
> 
> ## What changed
> 1. In `createAPIInstance.ts` and `request.ts`, the type `InternalAxiosRequestConfig` was replaced with `AxiosRequestConfig`. This change was made in the request interceptor and the request function.
> 2. In `GlobalNav.tsx`, the admin icon is now rendered directly as a component, instead of being imported as an image source.
> 
> ## How to test
> 1. For the Axios request configuration change, ensure that running the `tsc` command does not throw any errors and that tests are passing
> 2. For the admin icon change, check the global navigation and verify that the admin icon is displayed correctly.
> 
> ## Why make this change
> 1. The `InternalAxiosRequestConfig` type is not part of the public Axios API and should not be used. The `AxiosRequestConfig` type is the correct type to use for Axios request configuration.
> 2. Rendering the admin icon directly as a component was required because the icon was imported as a React component
</details>
  • Loading branch information
mkarajohn committed Mar 14, 2024
2 parents bf82559 + 5fe68f6 commit bf3c8eb
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 9 deletions.
8 changes: 3 additions & 5 deletions src/request/createAPIInstance.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import axios, { AxiosInstance, CancelTokenSource, type InternalAxiosRequestConfig } from 'axios';
import type { AxiosInstance, AxiosRequestConfig, CancelTokenSource } from 'axios';
import axios from 'axios';
import useRequestToken from '~/store/requestToken';
import { getTokenSilently, logoutAuth } from '~/utils/auth';
import { RequestProps, deleteToken, request, setToken, tokenFormat } from './request';
Expand All @@ -12,7 +13,6 @@ export type CreateAPIInstanceProps = {

export type CreateAPIInstanceType = {
instance: AxiosInstance;
// @ts-ignore
createRequest: <T = any>(
props: RequestProps
) => {
Expand All @@ -34,8 +34,6 @@ export const createAPIInstance = ({
},
hasAutomaticToken = true,
}: CreateAPIInstanceProps): CreateAPIInstanceType => {
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const orfiumAxios = axios.create({
baseURL: baseUrl,
});
Expand All @@ -57,7 +55,7 @@ export const createAPIInstance = ({
// if this fails then the user will be redirected to the response interceptor
// Fetching latest token is mandatory for all the request to have up-to-date information
orfiumAxios.interceptors.request.use(
async (config: InternalAxiosRequestConfig) => {
async (config: AxiosRequestConfig) => {

Check failure on line 58 in src/request/createAPIInstance.ts

View workflow job for this annotation

GitHub Actions / Deploy

Argument of type '(config: AxiosRequestConfig) => Promise<AxiosRequestConfig<any>>' is not assignable to parameter of type '(value: InternalAxiosRequestConfig<any>) => InternalAxiosRequestConfig<any> | Promise<InternalAxiosRequestConfig<any>>'.

Check failure on line 58 in src/request/createAPIInstance.ts

View workflow job for this annotation

GitHub Actions / Deploy

Argument of type '(config: AxiosRequestConfig) => Promise<AxiosRequestConfig<any>>' is not assignable to parameter of type '(value: InternalAxiosRequestConfig<any>) => InternalAxiosRequestConfig<any> | Promise<InternalAxiosRequestConfig<any>>'.

Check failure on line 58 in src/request/createAPIInstance.ts

View workflow job for this annotation

GitHub Actions / Deploy

Argument of type '(config: AxiosRequestConfig) => Promise<AxiosRequestConfig<any>>' is not assignable to parameter of type '(value: InternalAxiosRequestConfig<any>) => InternalAxiosRequestConfig<any> | Promise<InternalAxiosRequestConfig<any>>'.
if (hasAutomaticToken) {
const { token } = await getTokenSilently();
config.headers.Authorization = `Bearer ${token}`;

Check failure on line 61 in src/request/createAPIInstance.ts

View workflow job for this annotation

GitHub Actions / Deploy

'config.headers' is possibly 'undefined'.

Check failure on line 61 in src/request/createAPIInstance.ts

View workflow job for this annotation

GitHub Actions / Deploy

'config.headers' is possibly 'undefined'.

Check failure on line 61 in src/request/createAPIInstance.ts

View workflow job for this annotation

GitHub Actions / Deploy

'config.headers' is possibly 'undefined'.
Expand Down
6 changes: 3 additions & 3 deletions src/request/request.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import axios, { AxiosInstance, AxiosRequestConfig, InternalAxiosRequestConfig } from 'axios';

import type { AxiosInstance, AxiosRequestConfig } from 'axios';
import axios from 'axios';
import { axiosPromiseResult } from './utils';

const GET = 'get';
Expand Down Expand Up @@ -42,7 +42,7 @@ export const request =
...(onUploadProgress && { onUploadProgress }),
...(onDownloadProgress && { onDownloadProgress }),
responseType,
} as InternalAxiosRequestConfig;
} as AxiosRequestConfig;

const request = () => axiosPromiseResult<T>(orfiumAxios(config));

Expand Down
2 changes: 1 addition & 1 deletion src/ui/Navigation/components/GlobalNav/GlobalNav.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ function GlobalNavLink(props: GlobalNavLinkProps) {
return adminNavigationIsActive;
}}
>
<img alt={adminButtonTooltipText} src={AdminIcon} height={16} width={16} />
<AdminIcon alt={adminButtonTooltipText} height={16} width={16} />
</AppIconRRLink>
);
}
Expand Down

0 comments on commit bf3c8eb

Please sign in to comment.