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.

Sign up for GitHub

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

Jump to bottom

Feature Added Sidebar. #274

Open
wants to merge 20 commits into
base: develop
Choose a base branch
from
Open

Feature Added Sidebar. #274

wants to merge 20 commits into from

Conversation

ivinayakg
Copy link

@ivinayakg ivinayakg commented Jul 15, 2022 •
edited
Loading

A new react component is created. Sidebar is styled with css modules. Below is how it looks.
Web_capture_12-7-2022_113158_localhost
image

Here I hided the header links on the screen only which means they are still visible in the code. I did that to get the real look of the component.
Navigation is also routed with highlighting the active link.

@shubham-y
Copy link
Contributor

@ivinayakg Please add a screenshot of UI in Mobile mode as well.

@shubham-y shubham-y linked an issue Jul 21, 2022 that may be closed by this pull request
[UI Revamp] New sidebar #271
Open
MehulKChaudhari
MehulKChaudhari requested changes Jul 21, 2022
View reviewed changes
Copy link
Member

@MehulKChaudhari MehulKChaudhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that adding a sidebar to every page is good. We cannot add any animation to it as it will re-render upon changing the pages.

use it only once. If you don't have an idea please reach out.

__tests__/components/Sidebar.test.js Outdated Show resolved Hide resolved
components/Sidebar/index.jsx Outdated Show resolved Hide resolved
components/Sidebar/index.jsx Outdated Show resolved Hide resolved
components/Sidebar/index.jsx Show resolved Hide resolved
@ivinayakg
Copy link
Author

I don't think that adding a sidebar to every page is good. We cannot add any animation to it as it will re-render upon changing the pages.

use it only once. If you don't have an idea please reach out.

i had my own doubts about this, cause the navbar was getting rendered on every page, so I thought there is a reason behind this or some format that we are following.
but for rendering from one component i have to wrap this in the _app.js file's component right?

@ivinayakg
Copy link
Author

@MehulKChaudhari I have noted your requested changes and will revert you with them as soon as possible.
@shubham-y I don't have the design template of how it should look on mobile therefore it may look broken, on mobile view

@MehulKChaudhari
Copy link
Member

I don't think that adding a sidebar to every page is good. We cannot add any animation to it as it will re-render upon changing the pages.
use it only once. If you don't have an idea please reach out.

i had my own doubts about this, cause the navbar was getting rendered on every page, so I thought there is a reason behind this or some format that we are following. but for rendering from one component i have to wrap this in the _app.js file's component right?

Please ask if you have a doubt. And yes you have to wrap it in _app.js file

MehulKChaudhari
MehulKChaudhari requested changes Aug 22, 2022
View reviewed changes
Copy link
Member

@MehulKChaudhari MehulKChaudhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the logos are dark and some of them are light. Please make them consistent. and only dark them when active

components/Sidebar/index.jsx Show resolved Hide resolved
components/Sidebar/index.jsx Outdated Show resolved Hide resolved
akshay1502 and others added 5 commits August 22, 2022 22:42
@akshay1502 @shubham-y
@rohan09-raj @ivinayakg
Sync main with develop (Real-Dev-Squad#249)
7bb0bcd 
* 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 (Real-Dev-Squad#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>
@ivinayakg
added contstants and image
0d35066
@ivinayakg
fixed sidebar files
4d7da41
@ivinayakg
fixed the sidebar
3e19883
@ivinayakg
'Requested Changes Done'
f82eed7
ivinayakg
ivinayakg commented Aug 22, 2022
View reviewed changes
pages/index.js Outdated Show resolved Hide resolved
MehulKChaudhari
MehulKChaudhari requested changes Sep 6, 2022
View reviewed changes
Copy link
Member

@MehulKChaudhari MehulKChaudhari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @ivinayakg, Good work 🔥 But there are a few buttons that you haven't added so we cannot move ahead. Please refer to this link and make the changes.

https://www.figma.com/file/9t1Zr4PGQL4UWSIpeh4RMY/RDS?node-id=0%3A1

MehulKChaudhari and others added 4 commits December 4, 2022 23:11
@MehulKChaudhari
Add: Move to yarn (Real-Dev-Squad#289)
2f0e329 
* move to yarn

* fix: github ci command

* fix: github workflow commands

* add: test command in CI

* remove test command from ci
@MehulKChaudhari
*WIP* Add: cypress tests setup (Real-Dev-Squad#276)
955df28 
* add: cypress integration tests setup

* add mocks and config for .module.css file

* add package-lock.json to sync

* remove msw
@MehulKChaudhari
add: msw (Real-Dev-Squad#288)
fc04834 
* add: msw

* add: handlers

* fix: gitignore

* update yarn
@ranitasahaa1230
Feature/integrating-new-signup-flow (Real-Dev-Squad#293)
9feaa96 
feat: integrating signup flow
akshay1502 and others added 8 commits December 20, 2022 15:39
@akshay1502 @shubham-y
@rohan09-raj @ivinayakg
Sync main with develop (Real-Dev-Squad#249)
22d8795 
* 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 (Real-Dev-Squad#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>
@ivinayakg
added contstants and image
752bcf2
@ivinayakg
fixed sidebar files
1a89d49
@ivinayakg
fixed the sidebar
0a53637
@ivinayakg
'Requested Changes Done'
def0ffb
@ivinayakg
fixed typo
72bd58e
@ivinayakg
responsivness added
9cbf66b
@ivinayakg
Merge branch 'feature/new-sidebar' of https://github.com/ivinayakg/we…
125e025 
…bsite-crypto into feature/new-sidebar
@vercel
Copy link

vercel bot commented Dec 20, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
website-crypto ✅ Ready (Inspect) Visit Preview Dec 20, 2022 at 10:14AM (UTC)

@manish591 manish591 mentioned this pull request Jan 12, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prakashchoudhary07 prakashchoudhary07 Awaiting requested review from prakashchoudhary07

@rohan09-raj rohan09-raj Awaiting requested review from rohan09-raj

@shubham-y shubham-y Awaiting requested review from shubham-y

@MehulKChaudhari MehulKChaudhari Awaiting requested review from MehulKChaudhari

Requested changes must be addressed to merge this pull request.

Assignees

@ivinayakg ivinayakg

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[UI Revamp] New sidebar
5 participants
@ivinayakg @shubham-y @MehulKChaudhari @akshay1502 @ranitasahaa1230