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

refactor (Components) : Refactored Components #179

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Raunakk02
Copy link

Description

Refactored the files inside src/components folder

Changes

Implemented the changes as proposed in this comment of issue #108

How to test

Since the changes are just related to refactoring, simple testing of the UI components should be sufficient.

fixes #108

@Raunakk02 Raunakk02 changed the title Refactored Components refactor (Components) : Refactored Components Mar 10, 2022
@sjha2048
Copy link
Collaborator

Hi @Raunakk02 , can you please squash your commits and follow the commit message guidelines.

@Raunakk02
Copy link
Author

Hi @sjha2048, thanks for reviewing the PR. As suggested by you, I have squashed and updated the commits as per the commit message guidelines. Please let me know if any other change is needed.

@sjha2048 sjha2048 added needs review Need code review needs test Needs testing labels Mar 18, 2022
@GMishx
Copy link
Member

GMishx commented Mar 21, 2022

When I login, I see following error:

SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data
getLocalStorage
src/shared/storageHelper.js:59

  56 | // Get from localstorage
  57 | export const getLocalStorage = (key) => {
  58 |   if (typeof window !== "undefined") {
> 59 |     return JSON.parse(localStorage.getItem(key));
  60 |   }
  61 |   return null;
  62 | };

UserInfoDropdown
src/components/Header/Dropdowns/UserInfoDropdown.jsx:43

  40 | import { getNameInitials } from "shared/helper";
  41 | 
  42 | const UserInfoDropdown = () => {
> 43 |   const [currentGroup, setCurrentGroup] = useState(
  44 |     getLocalStorage("currentGroup") || getLocalStorage("user")?.default_group
  45 |   );
  46 |   const history = useHistory();

@Raunakk02
Copy link
Author

@GMishx please have a look at this PR created by @krishna9304. He has fixed the issue. It was out of the scope of this particular PR and some other contributors (including me) was facing it too.

@github-actions
Copy link

github-actions bot commented Apr 6, 2022

This pull request has conflicts, please rebase to resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the has merge conflicts PR has merge conflicts, require rebase label Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has merge conflicts PR has merge conflicts, require rebase needs review Need code review needs test Needs testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor large components
3 participants