-
Notifications
You must be signed in to change notification settings - Fork 65
Feat/add new dashboard UI #299
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
base: develop
Are you sure you want to change the base?
Feat/add new dashboard UI #299
Conversation
* Changes in navbar UI and content * Change in navbar UI * Change in navbar UI as per new design * Add login url from constants * Added changes to fetch and display data for signed-in users * Fixed display data for signed-out users * Added mobile UI change for currency exchange page * Added changes to use Link component for anchor tags * Remove commented code * css code refactor changes * Add and load paths from constants file * Remove !important property from navbar css * Refactor css code * Add UI changes in navbar * Add default avatar for user profile picture * Add UI changes in navbar * Add UI changes in navbar * Add profile link on greet message & error handling in fetch user * Fix profile url * Added .env.development file (RealDevSquad#245) Co-authored-by: user.name: "Rohan Raj Gupta <user.email: "rajrohan1914@gmail.com> * Improvement in code and changes to use cloudinary images * Add key prop * Replace useEffect by useLayoutEffect for login button * Improvement in navbar code * Update ecmaVersion in ESLint config * Revert useLayoutEffect changes for login button * Fix login button UI Co-authored-by: Shubham <yadav105shubham@gmail.com> Co-authored-by: Rohan Raj Gupta <78433013+rohan09-raj@users.noreply.github.com> Co-authored-by: user.name: "Rohan Raj Gupta <user.email: "rajrohan1914@gmail.com>
* Changes in navbar UI and content * Change in navbar UI * Change in navbar UI as per new design * Add login url from constants * Added changes to fetch and display data for signed-in users * Fixed display data for signed-out users * Added mobile UI change for currency exchange page * Added changes to use Link component for anchor tags * Remove commented code * css code refactor changes * Add and load paths from constants file * Remove !important property from navbar css * Refactor css code * Add UI changes in navbar * Add default avatar for user profile picture * Add UI changes in navbar * Add UI changes in navbar * Add profile link on greet message & error handling in fetch user * Fix profile url * Added .env.development file (RealDevSquad#245) Co-authored-by: user.name: "Rohan Raj Gupta <user.email: "rajrohan1914@gmail.com> * Improvement in code and changes to use cloudinary images * Add key prop * Replace useEffect by useLayoutEffect for login button * Improvement in navbar code * Update ecmaVersion in ESLint config * Revert useLayoutEffect changes for login button * Fix login button UI Co-authored-by: Shubham <yadav105shubham@gmail.com> Co-authored-by: Rohan Raj Gupta <78433013+rohan09-raj@users.noreply.github.com> Co-authored-by: user.name: "Rohan Raj Gupta <user.email: "rajrohan1914@gmail.com>
…bsite-crypto into feature/new-sidebar
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@manish591 can't find the test in the PR. Are the test already merged 🤔. |
No, the tests are not merged yet. Will write them now |
samyakshah3008
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid work Manish!
Here are few pointers which you can refactor:
- Instead of using px, use rem.
- Don't use !important
- Avoid white spaces
Added few single line comments, rest is all good for approval.
heyrandhir
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @manish591! .Would recommend you creating small and incremental PRs in the future. Thank you!
ok |
| display: flex; | ||
| justify-content: space-between; | ||
| padding: 1.6rem; | ||
| background-color: var(---bg-blue-300); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appears to be a CSS variable name typo in the sidebar mobile toggle styling. The property background-color: var(---bg-blue-300); has an extra dash - it should be background-color: var(--bg-blue-300); (with two dashes instead of three). This will ensure the mobile toggle background color renders correctly.
| background-color: var(---bg-blue-300); | |
| background-color: var(--bg-blue-300); |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| @@ -0,0 +1,5 @@ | |||
| export default { | |||
| ICON_PATH_ARROW: '/assets/rrow.svg', | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There appears to be a typo in the icon path for the arrow. The current path is '/assets/rrow.svg' but should likely be '/assets/arrow.svg' or '/assets/MenuArrow.svg' (which exists in the public assets directory based on the PR). This could cause the arrow icon to not load properly in the UI.
| ICON_PATH_ARROW: '/assets/rrow.svg', | |
| ICON_PATH_ARROW: '/assets/MenuArrow.svg', |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
| 'June', | ||
| 'July', | ||
| 'August', | ||
| 'Sepetember', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a typo in the month name - 'Sepetember' should be spelled 'September'. This appears in the months list constant that's used for the transaction dropdown.
| 'Sepetember', | |
| 'September', |
Spotted by Diamond
Is this helpful? React 👍 or 👎 to let us know.
fixes #272
NOTE: I have taken a pull from Vinayak's PR #274 that is already been reviewed.
Visual Reference