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

Remove full name entry + hide in form. #2757

Merged
merged 4 commits into from
Nov 12, 2018

Conversation

Anish-Agnihotri
Copy link
Contributor

@Anish-Agnihotri Anish-Agnihotri commented Nov 10, 2018

Description

This is a simple commit that simply defaults the details.html page to take username as default value rather than full name, and removes the option to add a full name altogether from new.html. It's a simple commit which does this through:

  1. Removing the option to add a name.
  2. Defaulting occurrences of bounty_owner_name to result.bounty_owner_github_username in bounty_details.js. Since the option to anonymize full name has been removed altogether, I've also taken out the code that was committed previously for hide funder details when funder selects to be Anonymous #2273.
Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

User Interface.

Testing

Tested cross-platform as usual and a few times with various issues on Rinkeby to ensure function.

Refers/Fixes

Fixes #2748, removes need for #2273.

CC: @frankchen07

Copy link
Member

@thelostone-mc thelostone-mc left a comment

Choose a reason for hiding this comment

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

Could we do away with the shared/name.html ( any styling associated with it )

Also the funder we should the email if the funder chooses to keep that data hidden.
I've covered those as a part of #2759 ?

could you undo the changes at bounty_details.jselse we'll end up with conflicts ? 😅

@thelostone-mc thelostone-mc added enhancement This is a feature to be enhanced or improved. frontend This needs frontend expertise. labels Nov 10, 2018
@codecov
Copy link

codecov bot commented Nov 11, 2018

Codecov Report

Merging #2757 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2757   +/-   ##
=======================================
  Coverage   28.89%   28.89%           
=======================================
  Files         163      163           
  Lines       13080    13080           
  Branches     1753     1753           
=======================================
  Hits         3780     3780           
  Misses       9193     9193           
  Partials      107      107

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39af78e...6a39622. Read the comment docs.

@Anish-Agnihotri
Copy link
Contributor Author

Ah yes. Your pr at #2759 is certainly more robust than this in every way, haha.

There are no styles associated with name.html file as far as I know so that shouldn't be a problem. Although, fullName as a var is mentioned in a few files, let me know if you want me to remove that.

Other than that, I'd be glad to simply delete this PR altogether, and you can just add the one-liner to remove shared/name.html from the new issue page yourself, so that the code is consistent.

@thelostone-mc
Copy link
Member

@Anish-Agnihotri added it into this PR ^_^

@thelostone-mc thelostone-mc merged commit 7da5896 into gitcoinco:master Nov 12, 2018
@Anish-Agnihotri Anish-Agnihotri deleted the remove_fullname branch November 12, 2018 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is a feature to be enhanced or improved. frontend This needs frontend expertise.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants