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.
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.
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.
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.
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.
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