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

Profile Card UI Updated #205

Open
wants to merge 3 commits into
base: development
Choose a base branch
from

Conversation

blyncnov
Copy link

@blyncnov blyncnov commented Oct 7, 2022

Pull Request Preview

  • Mobile View

localhost_3000_profile(iPhone X)

  • Large Screen

Screenshot 2022-10-07 at 20 16 40 (2)
Screen

@d-e-v-esh
Copy link
Contributor

Looks really good 🔥🔥
BTW you should remove the yarn.lock from staged changes before making a commit, otherwise it will overwrite the master yarn.lock file. The commit should contain only the files where you changed the code.

The quick solution will be that you can undo this commit, remove the yarn.lock file from the staged changes. then make back the commit and then do git push -f.

@d-e-v-esh
Copy link
Contributor

Also I would suggest that you make multiple smaller commits as you are adding or removing things as opposed to one large commit. It will make it easier to look through and track back if needed in future.

@blyncnov
Copy link
Author

blyncnov commented Oct 7, 2022

Alright , will take note of those ..

Literally you mean, if I make changes in profile.js , then only profile.js should be in my commit ?

@blyncnov
Copy link
Author

blyncnov commented Oct 7, 2022

You can Review again

@d-e-v-esh
Copy link
Contributor

Literally you mean, if I make changes in profile.js , then only profile.js should be in my commit ?

Yes! Exactly.

@d-e-v-esh
Copy link
Contributor

The yarn.lock is still in this Pull Request. This number at the top right is inflated becasue of the changes in the yarn.lock. I think you will have to undo commit.
image

You will find lots of tutorials on YouTube on this topic that will help.

<div className='profile-image-circle'></div>
</ProfileImageContainer>
<ProfileCTXText>
<div className='profile-ctx-user'>
Copy link
Member

Choose a reason for hiding this comment

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

Use styled components here instead of using h1/div tags.

Copy link
Member

@padmajabhol padmajabhol left a comment

Choose a reason for hiding this comment

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

Please use styled components and convert the div and h1s. Also please delete the package -lock.json. We don't need to mix both npm and yarn packages. The UI looks good. Thanks a lot.

@blyncnov
Copy link
Author

blyncnov commented Oct 8, 2022

I will do that

@blyncnov blyncnov force-pushed the profile-card-ui branch 2 times, most recently from ba91dcc to 625b286 Compare October 8, 2022 11:43
@blyncnov
Copy link
Author

blyncnov commented Oct 8, 2022

Changes Made... I will wait back for your review.

Thanks

@d-e-v-esh
Copy link
Contributor

@blyncnov You'll have to undo the commits, yarn lock is still in the Pull Request.
image

@blyncnov
Copy link
Author

blyncnov commented Oct 9, 2022

I thought I deleted that already.

Screenshot 2022-10-09 at 07 41 53

@blyncnov
Copy link
Author

blyncnov commented Oct 9, 2022

@d-e-v-esh maybe you can show me how else if you are free ??

Initially I uncommitted and push the file, I can't find the yarn.lock in my branch of repo tho and in the PR, i see deleted

@d-e-v-esh
Copy link
Contributor

Portfolio.-.Visual.Studio.Code.2022-10-09.14-09-04-1.mp4

@blyncnov I made a small video where I removed an image from a commit and re commited and pushed.

@blyncnov
Copy link
Author

blyncnov commented Oct 9, 2022

20221009_095444.jpg

Great.. did that

@blyncnov
Copy link
Author

blyncnov commented Oct 9, 2022

Completed 👋

@d-e-v-esh
Copy link
Contributor

@blyncnov Great! Can you please rename the commit from test removal to something more descriptive?

@blyncnov
Copy link
Author

blyncnov commented Oct 9, 2022

@d-e-v-esh Okay . Will do

@blyncnov
Copy link
Author

blyncnov commented Oct 9, 2022

Did that work @d-e-v-esh ??

@d-e-v-esh
Copy link
Contributor

No, descriptive meaning something that tells more about that you added or removed from the codebase

@blyncnov
Copy link
Author

blyncnov commented Oct 9, 2022

@d-e-v-esh

I should do that via the commit message?

e.g

git commit -m "I improved user profile UI Card design bla bla bla "

Right ??

@d-e-v-esh
Copy link
Contributor

@blyncnov Commit message has two parts, title and description. Look them up.

I Updated the user profile card UI Design,  and made edits, Styled components based styling
@blyncnov
Copy link
Author

blyncnov commented Oct 9, 2022

@d-e-v-esh please check

It's my first time with this really, and I am learning alot from you ..

Thanks

@d-e-v-esh
Copy link
Contributor

@blyncnov You can yourself check under the commits section.
image

You see there are 3 commits, we want to have all the changes under one commit which is the latest one you made. I think you should undo all commits and re-commit it with the new commit message. And remove yarn lock from it. You don't have to wait for me to check. You can yourself check under the commits tab in the Pull Request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants