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

Implemented Issue 629 #685

Merged
merged 1 commit into from Oct 14, 2012
Merged

Implemented Issue 629 #685

merged 1 commit into from Oct 14, 2012

Conversation

gavinlaking
Copy link
Contributor

Added preference to user's profile page (if they have a Twitter account connected), allowing the super control of the 'Post to: Twitter' checkbox in the main interface.

Added preference to user's profile page (if they have a Twitter account connected), allowing the super control of the 'Post to: Twitter' checkbox in the main interface.
@carols10cents
Copy link
Contributor

I've started taking a look at this but i've run out of time, I will hopefully be able to finish tomorrow though!! I'm excited to have this and I bet @whit537 will be too :)

@carols10cents carols10cents merged commit 0e81d20 into hotsh:master Oct 14, 2012
@carols10cents
Copy link
Contributor

Awesome, merged!! Thank you!!! ❤️ ❤️ ❤️

I made a few changes after your commit that you might want to take a look at:

  • I wrote some tests for this so that we don't break this in the future (42ed2e8)
  • Made the user attribute and checkbox name the same because I couldn't remember which was which and I like consistency, really picky though (7637705)
  • Refactored to use rails' form helpers-- I think this view is from back when we were using Sinatra instead of Rails and didn't have access to its nice form helper methods that make it easy to use checkboxes for properties (5297e46)
  • Started a new section in the markup for preferences (I'm guessing we'll have more) and make the checkbox align with the other inputs (7fd7fcb)

I'll be adding you to the about page soon, and I'm going to add you to the team of committers as well! Please read the section in the readme about being a committer and let me know if you have any questions ❤️❤️

@gavinlaking
Copy link
Contributor Author

Hi Carol,

Thanks for the merge, and updates! I didn't write tests on this particular piece of functionality because I wasn't really sure where to start with that. I also wanted to refactor my code (and some of the surrounding code) for consistency but refrained as I didn't want to stand on toes.

Looking forward to contributing more, and if you say I can, I'll even tidy up a bit too! :-)

Gav

@carols10cents
Copy link
Contributor

Cleaning up is always welcome!!! ❤️ ❤️ There are definitely places that could use it.

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.

None yet

2 participants