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

Style checkbox for hiding email in user profile. #679

Merged
merged 1 commit into from May 16, 2014

Conversation

Projects
None yet
5 participants
@knappe
Copy link
Contributor

knappe commented Apr 22, 2014

This was unstyled and needed some love.

@adammcarth

This comment has been minimized.

Copy link

adammcarth commented Apr 22, 2014

Nice. Do you have a screenshot?

I've been working on this, I'll integrate your changes if it looks better...

RubyGems Profile Design

@knappe

This comment has been minimized.

Copy link
Contributor

knappe commented Apr 22, 2014

Nothing terribly innovative. I do like having the checkbox on the left and centered with the text though.

I like your changes to make the profile more compact @adammcarthur!

5

@adammcarth

This comment has been minimized.

Copy link

adammcarth commented Apr 22, 2014

Thank you! I'm just finishing the new translations off, and I'll throw a pull request up later this week with my changes.

Your checkbox is much better than how it is now, but as you pointed out, I kind of like the idea of bringing everything together a bit more. There's so much wasted space at the moment, particularly where the avatar field is.

Maybe we can get a third opinion on whose checkbox works the best? :)

@parkr

This comment has been minimized.

Copy link
Contributor

parkr commented Apr 22, 2014

I think we need a better indicator that the email is hidden, actually. @adammcarthur, I'd go with your solution if the text box where my email goes also changed – perhaps becoming italic or greying out a bit? Just showing that it's "hidden" through more than just the checkbox.

@adammcarth

This comment has been minimized.

Copy link

adammcarth commented Apr 22, 2014

Hey @parkr,

Maybe a strikethrough?

@knappe

This comment has been minimized.

Copy link
Contributor

knappe commented Apr 22, 2014

I'm confused on why having the checkbox checked is not indication enough that a setting is in place. That is convention.

The email address is used for other purposes besides displaying it on the profile page. Does greying it out also mean that the other functions are disabled?

@adammcarth

This comment has been minimized.

Copy link

adammcarth commented Apr 22, 2014

I do quite like the idea of expressing it through design.

Perhaps we remove the checkbox completely and place a button inside the text field (off the right) that changes from "Hide from Profile" to "Show in Profile" - fading the box appropriately on click.

I might have a shot at that tomorrow - but I just realised I've got school in 6 hours so goodnight all 👍

@parkr

This comment has been minimized.

Copy link
Contributor

parkr commented Apr 22, 2014

Does greying it out also mean that the other functions are disabled?

Well, of course not, but I'm the guy who suggested that behaviour. The checkbox would be ok, I guess, but it doesn't feel quite obvious enough to me in the more compact position that @adammcarthur proposed.

@adammcarth

This comment has been minimized.

Copy link

adammcarth commented Apr 22, 2014

Then again, is the significance of this feature being overlooked in the wider scheme of things? I consider something such as hiding an email address to be entirely user driven, that is, they will seek the feature out if they really consider it necessary - in which case it wouldn't have to stand out THAT much. If they want it, they'll find it, right?

Seriously I'm going this time ;)

@knappe

This comment has been minimized.

Copy link
Contributor

knappe commented Apr 30, 2014

@adammcarthur We're approaching 7 days. I'm in favor of merging this now and then you can pull in the change to your branch. I would prefer keep the styling with the checkbox to the left of the label.

@dwradcliffe Thoughts on this PR?

@adammcarth

This comment has been minimized.

Copy link

adammcarth commented Apr 30, 2014

Yeah, sorry. I've been caught up with homework, and some other stuff has come up in the mean time. It will be some time until my changes are released.

Adam.

@bf4

This comment has been minimized.

Copy link
Contributor

bf4 commented May 4, 2014

A little OT:
So, the weird thing about this feature, in general, is that it is about hiding the email address in your rubygems.org account. It has nothing to do with email addresses in gemspecs. And, as a gem maintainer, it can be very useful to add a co-maintainer by just looking them up. Maybe the email should only be hidden to unregistered users?

@adammcarth

This comment has been minimized.

Copy link

adammcarth commented May 4, 2014

You've made some good points here @bf4.

I still don't think it's fair to hand out users private email addresses without giving them the option of disabling it. The reality is, some people just don't want to be contacted.

Granted, most people are using the same public email they use in Gemspecs, but you have to also account for people who have signed up and don't want their email exposed.

Again, hiding emails from non-registered users is probably good practise, but I don't see the need for it here.

@knappe

This comment has been minimized.

Copy link
Contributor

knappe commented May 4, 2014

The original PR was a request to hide email addresses from spammers. Yes,
the email can be pulled from the .gemspec, but it does require more work
than scraping profile pages.
On May 4, 2014 5:17 PM, "Adam McArthur" notifications@github.com wrote:

You've made some good points here @bf4 https://github.com/bf4.

I still don't think it's fair to hand out users private email addresses
without giving them
the option of disabling it. The reality is, some people just don't want to
be contacted.

Granted, most people are using the same public email they use in Gemspecs,
but you have to also account for people who have signed up and don't want
their email exposed.

Again, hiding emails from non-registered users is probably good practise,
but I don't see the need for it here.


Reply to this email directly or view it on GitHubhttps://github.com//pull/679#issuecomment-42147394
.

@knappe

This comment has been minimized.

Copy link
Contributor

knappe commented May 16, 2014

@dwradcliffe Can we get this merged?

dwradcliffe added a commit that referenced this pull request May 16, 2014

Merge pull request #679 from knappe/master
Style checkbox for hiding email in user profile.

@dwradcliffe dwradcliffe merged commit 50a4699 into rubygems:master May 16, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@dwradcliffe

This comment has been minimized.

Copy link
Member

dwradcliffe commented May 16, 2014

Thanks!

@dwradcliffe

This comment has been minimized.

Copy link
Member

dwradcliffe commented May 16, 2014

Deployed!

dwradcliffe added a commit that referenced this pull request Jan 29, 2015

Merge pull request #679 from knappe/master
Style checkbox for hiding email in user profile.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment