Closed esmale closed 11 years ago
A few comments.
The validation on the profile form is fine, I don't have any complaints about that!
I don't really want to do a profile show page, as I specifically wanted to avoid emphasizing the members more than what we currently have. The point of the site isn't really to profile all the members of the community, as I expect people to come and go. It's more just a small directory with basic contact info and links.
So I'd prefer that we just do a truncate of the profile text. I've already seen a few people update their profiles down to a shorter character limit so I don't think that's a big deal or an unreasonable expectation.
I see you updated the Team page to follow the same markup as the Members page, but I'd actually prefer to keep the layouts separate with the single column vertical layout on the team page. We shouldn't ever have a big core team for Dallas.rb and I liked the way that page flowed with a single column.
Thanks!
Fair enough, just wanted to try some things out.
Locally, I have the team page back to the way it was before, and I've pulled out the Profile show page.
The only thing left is the blurb migration. I'm wrestling a bit with how long the blurb should be. In my experiments with the current size of the Profile card, 90 characters is the most that could really fit, but almost everyone that has put something into the blurb field has entered more than 90 characters. Even yours, @hkarthik , is 95 characters long.
As much as I argued, and still think, that the 2 column layout makes better use of the space...I'm starting to think the 1 column layout would be better. Thoughts?
Ah that's too bad. I actually like the card format a lot, but now that I think about it I've mostly seen this kinda thing used when the content sizes are mostly constant with less variability.
What if we made each card just a little taller and made each page a 4x2 instead of a 5x2? Would that help?
That might work. I'll give it a try and see how it looks. Making the cards a little taller (tall enough for 1 more line of text) might give us enough room for it to work. Will have to see how the overall effect looks/feels, especially since most people don't have anything in the blurb (so that might be a lot of empty space).
Cool, feel free to attach a screenshot to this PR. Makes it a little easier than trying to setup locally.
One extra line of text works great for my blurb (101 characters) and Geof's (143 characters), but is still not enough for Steve's (total length 141, displaying 120) and Matt's (total length 170, displaying 136).
Two extra lines seems to be enough for everyone in the current scenario. I do start to wonder, though, if that's too much blank space for people that do not have a blurb. And it would still leave us to figure out where to set the character limit. What do you think @hkarthik ?
I say we go with 3 lines and set a limit of 140 chars. Since Twitter I think 140 chars is a good standard. I agree 4 lines is a little too much, especially if people are leaving the blurb blank.
Aww man! Now I have to go back to using my twitter profile. Drat! :wink: — Sent from Mailbox for iPhone
On Sun, May 12, 2013 at 3:54 PM, Karthik Hariharan notifications@github.com wrote:
I say we go with 3 lines and set a limit of 140 chars. Since Twitter I think 140 chars is a good standard. I agree 4 lines is a little too much, especially if people are leaving the blurb blank.
Reply to this email directly or view it on GitHub: https://github.com/dallasrb/dallasrb-web/pull/30#issuecomment-17785150
@esmale Let me know when you think we're good to merge. I'll review locally at that point. Thanks for following through on this!
Unless I'm completely forgetting something that I was supposed to do/undo, I think we are ready to go.
Requiring profile name on updates, but not on creation since Profile creation happens during the github authentication process. Once the User/Profile is in the system, blank names are no longer allowed.
I admit that the error display for the Profile editing form is not very clean or DRY. The default HTML generated by the text_field method and the Twitter bootstrap HTML structure don't mix well together. I'm sure there's a form-related gem that would generate Bootstrap compatible HTML, but I'm pressed for time at the moment and wanted to get this pull request up, at least for feedback.
Instead of restricting the blurb length via migration and asking poor Steve to shorten his bio, I thought we could just truncate it and provide a link to a Profile#show page that displays the full text.