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

feat : added links to contribution guidelines and COC #125

Closed
wants to merge 4 commits into from
Closed

feat : added links to contribution guidelines and COC #125

wants to merge 4 commits into from

Conversation

nlok5923
Copy link
Member

@nlok5923 nlok5923 commented Sep 24, 2020 •
edited
Loading

Fixes

Description

added links to contribution guidelines and code of conduct.

Screenshot Before Change:

Screenshot (1356)

Screenshot After Change:

Screenshot (1355)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Non Functional Requirement

  • Follows the code style of this project.
  • Tests Cover Changes
  • I have performed a self-review of my own code
  • All new and existing tests passed.
  • Documentation

@nlok5923
Copy link
Member Author

nlok5923 commented Sep 24, 2020 •
edited
Loading

deployment link : https://nlok5923-getmein-web-5.glitch.me/

Portal to get an invite to IIITV Github organization

@nlok5923 nlok5923 changed the title added links to contribution guidelines and COC feat : added links to contribution guidelines and COC Sep 24, 2020
@nlok5923
Copy link
Member Author

@iiitv/project-maintainers sir please review this pull request.

aashutoshrathi
aashutoshrathi requested changes Sep 24, 2020
View reviewed changes
views/index.html Outdated
type="submit" onclick="showToast()">Submit</button><br />
<span><a class="uk-text-meta uk-text-center uk-link-reset" target ="_blank" href="https://github.com/iiitv/getmein-web/blob/master/CODE_OF_CONDUCT.md">CODE OF CONDUCT</a>&emsp;
<a class="uk-text-meta uk-text-center uk-link-reset uk-text-nowrap" target ="_blank" href="https://github.com/iiitv/getmein-web/blob/master/CONTRIBUTING.md">CONTRIBUTION GUIDELINES</a>
</span>
Copy link
Member

Choose a reason for hiding this comment

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

Few changes:

  • Add uk-button to anchor tags
  • Wrap both buttons in a div and make it display: inline instead of keeping it in a span

views/index.html Outdated
type="submit" onclick="showToast()">Submit</button>
type="submit" onclick="showToast()">Submit</button><br />
<span><a class="uk-text-meta uk-text-center uk-link-reset" target ="_blank" href="https://github.com/iiitv/getmein-web/blob/master/CODE_OF_CONDUCT.md">CODE OF CONDUCT</a>&emsp;
<a class="uk-text-meta uk-text-center uk-link-reset uk-text-nowrap" target ="_blank" href="https://github.com/iiitv/getmein-web/blob/master/CONTRIBUTING.md">CONTRIBUTION GUIDELINES</a>
Copy link
Member

Choose a reason for hiding this comment

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

&emsp is hacky instead use uk-margin-right-small

@nlok5923
Copy link
Member Author

After changes :
Screenshot (1357)

@aashutoshrathi
Copy link
Member

Add uk-button-default too. Those should be buttons

@nlok5923
Copy link
Member Author

After changes :
Screenshot (1358)

@nlok5923
Copy link
Member Author

nlok5923 commented Sep 25, 2020 •
edited by unfurl-links bot
Loading

deployment link ; https://nlok5923-getmein-web-5.glitch.me/

aashutoshrathi
aashutoshrathi requested changes Sep 25, 2020
View reviewed changes
Copy link
Member

@aashutoshrathi aashutoshrathi left a comment

Choose a reason for hiding this comment

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

image

This is how it will look like after changes

views/index.html
type="submit" onclick="showToast()">Submit</button><br />
<div class="repo-links">
<a class="uk-text-meta uk-text-center uk-link-reset uk-button uk-button-default uk-margin-right-small" target ="_blank" href="https://github.com/iiitv/getmein-web/blob/master/CONTRIBUTING.md">CONTRIBUTION GUIDELINES</a>
<a class="uk-text-meta uk-text-center uk-link-reset uk-button uk-button-default" target ="_blank" href="https://github.com/iiitv/getmein-web/blob/master/CODE_OF_CONDUCT.md"> CODE OF CONDUCT</a>
Copy link
Member

Choose a reason for hiding this comment

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

Change classes for both buttons to

uk-text-center uk-button uk-button-default uk-border-pill

public/style.css
@@ -64,6 +64,10 @@ footer > a {
color: #bbbbbb;
}

.repo-links {
display: inline;
Copy link
Member

Choose a reason for hiding this comment

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

Add margin: 1rem

@nlok5923
Copy link
Member Author

@aashutoshrathi sir After changes it looks like :
Screenshot (1365)

@aashutoshrathi
Copy link
Member

Make sure the spacing is correct here

@nlok5923
Copy link
Member Author

sir on adding spacing those button move over the footer as footer position : absolute as well as there no more space for footer to shift below while maintaining 100vh
Screenshot (1368)

@AmanRaj1608
Copy link
Member

AmanRaj1608 commented Sep 26, 2020 •
edited
Loading

@nlok5923 device size?
I guess this should be fine for all devices
For device height lower than 650 we have to use media query

https://loutish-chain.surge.sh/

@nlok5923
Copy link
Member Author

nlok5923 commented Sep 26, 2020 •
edited
Loading

@nlok5923 device size?
I guess this should be fine for all devices
For device height lower than 650 we have to use media query
https://loutish-chain.surge.sh/

Sir but in this implementation too https://loutish-chain.surge.sh/ button overlap with footer in desktop view.

**Welcome to IIITV**Portal to get an invite to IIITV Github
organization

my device size : 1366x768

Portal to get an invite to IIITV Github organization

theyashshahs
theyashshahs requested changes Sep 26, 2020
View reviewed changes
Copy link
Member

@theyashshahs theyashshahs left a comment •
edited
Loading

Choose a reason for hiding this comment

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

WhatsApp Image 2020-09-26 at 10 47 18 PM

In mobile view, there is no space between the two buttons

@nlok5923 nlok5923 closed this Sep 27, 2020
@nlok5923 nlok5923 mentioned this pull request Sep 27, 2020
Closed
8 tasks
@theyashshahs
Copy link
Member

@nlok5923 why did you close this PR and opened a new work, try to work on the same PR

@nlok5923
Copy link
Member Author

@yashshah2820 sorry sir for making a new for for the same issue . But sir there were some issues with my machine with the file location that's why I had to open a new pr.

Sir I will not repeat the same mistake again.

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

@theyashshahs theyashshahs theyashshahs requested changes

@aashutoshrathi aashutoshrathi Awaiting requested review from aashutoshrathi

@AmanRaj1608 AmanRaj1608 Awaiting requested review from AmanRaj1608

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@nlok5923 @aashutoshrathi @AmanRaj1608 @theyashshahs