Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: admin icon in sidenav #138

Merged
merged 2 commits into from
Mar 14, 2024
Merged

fix: admin icon in sidenav #138

merged 2 commits into from
Mar 14, 2024

Conversation

mkarajohn
Copy link
Contributor

@mkarajohn mkarajohn commented Mar 13, 2024

Description

Screenshot

Test Plan


Generated summary (powered by Graphite)

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

Copy link
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @mkarajohn and the rest of your teammates on Graphite Graphite

@mkarajohn mkarajohn marked this pull request as ready for review March 13, 2024 17:16
@mkarajohn mkarajohn merged commit bf3c8eb into master Mar 14, 2024
4 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants