Skip to content

adding icons in contact page #96

Merged
s-ayush2903 merged 5 commits intoNJACKWinterOfCode:masterfrom
Vishal-raj-1:master
Jan 6, 2021
Merged

adding icons in contact page #96
s-ayush2903 merged 5 commits intoNJACKWinterOfCode:masterfrom
Vishal-raj-1:master

Conversation

@Vishal-raj-1
Copy link
Contributor

@Vishal-raj-1 Vishal-raj-1 commented Jan 4, 2021

Issue Fix

Fixes #92

Description of changes made
I have adding the icons which look nice both in desktop and mobile view. I have adding some hover effect as well. I have centralize the images. (check video for more info)

Submissions guide:

  • Have you made corresponding changes to the documentation?
  • Your submission doesn't break any existing feature.
  • Have you lint your code locally prior to submission?

Video

Contact._.NJACK.Winter.of.Code.-.Google.Chrome.2021-01-04.19-55-18.mp4

@s-ayush2903 @Ankit7Das Sir Please Review my PR !!

@vercel
Copy link

vercel bot commented Jan 4, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/s-ayush2903/nwoc-website-alpha-version/j4n9ftxta
✅ Preview: https://nwoc-website-alpha-version-git-fork-vishal-raj-1-master.s-ayush2903.vercel.app

@s-ayush2903
Copy link
Member

@Vishal-raj-1 This is not how it works, in order to open a PR you have to make sure that no one else is working on it and seek approval by maintainer to work on the issue, you never commented on the issue that you want to work on it and it was not good to assign you as neither had commented or mentioned anywhere in the issue about your interest in it moreover, one more participant was involved in the corresponding issue which displayed their interest/willingness to work on it. This isn't for this repo only, this is a rule of thumb in open source projects

@Vishal-raj-1
Copy link
Contributor Author

Vishal-raj-1 commented Jan 4, 2021 via email

Copy link
Member

@s-ayush2903 s-ayush2903 left a comment

Choose a reason for hiding this comment

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

  • You can definitely use better(and visible) icon for gitter, use gmail icon for gmail address and the normal mail icon for the webmail(the one with domain name @iitp.ac.in)

Also the current UX doesn't look pleasant to me, any thoughts do you have and I'm pinging here @Ankit7Das as well for input on the same, because those icons below pictures of organizers look a bit weird(because of their noticably large size)

@s-ayush2903
Copy link
Member

s-ayush2903 commented Jan 4, 2021

Also @Vishal-raj-1 you needn't mark the work as done unless you've done it in any commit, you should uncheck the appropriate checkboxes(in the PR description) : )

@Vishal-raj-1
Copy link
Contributor Author

Vishal-raj-1 commented Jan 5, 2021

  • You can definitely use better(and visible) icon for gitter, use gmail icon for gmail address and the normal mail icon for the webmail(the one with domain name @iitp.ac.in)

ok sure !!

Also the current UX doesn't look pleasant to me, any thoughts do you have and I'm pinging here @Ankit7Das as well for input on the same, because those icons below pictures of organizers look a bit weird(because of their noticably large size)

So I can do one thing, will do text align left(like before) and decrease the font-size of facebook and github icons.

@Vishal-raj-1
Copy link
Contributor Author

Also @Vishal-raj-1 you needn't mark the work as done unless you've done it in any commit, you should uncheck the appropriate checkboxes(in the PR description) : )

updated !!

@Vishal-raj-1
Copy link
Contributor Author

@s-ayush2903 @Ankit7Das I have resolved the issues !! Please review it 🔥

@s-ayush2903
Copy link
Member

The UI looks fine, good work 👍
Now that I go through the files changed I can see the same indentation changes that ain't needed(review same as #102 (review)), we'll configure the repository for the contributions entirely via adding a global codestyle in the project which will allow people to format the file according to the lints configured in the repository(that'll be done as the program finishes and we get back on website to improve it for next edition), but for now you'll have to revert irrelevant changes on your own sorry for that

@Vishal-raj-1
Copy link
Contributor Author

@s-ayush2903 @Ankit7Das I have restore the previous indentation. Please review it !! 🔥

@Vishal-raj-1
Copy link
Contributor Author

#102 will solve this issue as well !! I am closing this PR.

@s-ayush2903
Copy link
Member

@Vishal-raj-1 You were not supposed to close it, this is fine, we can merge it. I'm reopening it 👍

@s-ayush2903 s-ayush2903 reopened this Jan 6, 2021
Copy link
Member

@s-ayush2903 s-ayush2903 left a comment

Choose a reason for hiding this comment

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

LGTM

@s-ayush2903 s-ayush2903 merged commit d87df08 into NJACKWinterOfCode:master Jan 6, 2021
@Vishal-raj-1
Copy link
Contributor Author

@Vishal-raj-1 You were not supposed to close it, this is fine, we can merge it. I'm reopening it 👍

Sir I have closing this PR !! Because what I have done is solving one issue and make one PR and solving another issue on another branch. But I am opening new branch after opening the PR !! from the next time I will open the branch first then do changes and make a PR from that branch and start working on another issue on another branch.

Let me clarify you what happen here !!

This PR solve the issues of adding icons in contact page, #101 will solve this issue as well drop down list issues and #102 will solve this two issues and search bar.

sorry for the inconvenience. I was stuck in branching. I can do rebasing as well(fixup and squashing commits) but I feel there is no need to do that because it will deploy with the last commit that we have done on PR !!

@Vishal-raj-1
Copy link
Contributor Author

LGTM

Thank you for merging !! Feeling relaxed I was totally stuck because of that indentation.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

showing icons instead of links

2 participants