Misc tweaks for your review #467

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
4 participants
Contributor

SaqibS commented Mar 30, 2012

My first contribution to the project - found a couple of features in the issue list which I've wanted myself. Both are view-only changes, and so there are no unit tests (don't know if you want to use Watin, etc?). Some further CSS work may be needed to style the social buttons. Feedback appreciated.

  1. Put package name in title of package (issue #353)
  2. Put social like and share buttons on the package page (issue #446)

@pranavkm pranavkm commented on the diff Apr 25, 2012

Website/Views/Packages/DisplayPackage.cshtml
@@ -208,4 +216,8 @@
}
</tbody>
</table>
+
+ <div id="socialShare">
+ @LinkShare.GetHtml(pageTitle: Model.Title, linkSites: new LinkShareSite[] { LinkShareSite.Facebook, LinkShareSite.Twitter, LinkShareSite.Delicious })
@pranavkm

pranavkm Apr 25, 2012

Member

Good grief. People actually use this :)

@half-ogre

half-ogre Apr 30, 2012

Contributor

Do these work? I've never actually used these helpers. Twitter and Facebook seem good, but does anyone use Delocious anymore?

Contributor

half-ogre commented Apr 30, 2012

Any reason you only added search and main landmark roles? We could probably also add navigation, but I don't feel strongly about it.

Contributor

half-ogre commented Apr 30, 2012

@SaqibS Can you include a screen capture of what the social buttons look like on the package page? When changing UI, it's always helpful to include images if your changes, so reviewers don't have to pull until for the initial review. I typically use imgur for throw-away screen captures like this.

If including a capture is a problem, I can go ahead and pull to check out the changes.

Aside from that, these changes look good. Thanks for contributing, and after we look at the UI changes, we can pull it in.

Contributor

half-ogre commented May 2, 2012

I pulled the changes to the see the UI changes. Here's what it looks like:

social UI changes to package page

On the top sectio, the spacing is all off. On the bottom, the Delicious link's image isn't loading.

In general, I think I'd like a smaller, simple button for the social buttons in just one place on the page. Having it on the version section seems like to much. Could we just put them on the page once, at the top of the page, and use the smaller buttons like were being used at the bottom?

Contributor

half-ogre commented Jun 12, 2012

@SaqibS Would you like to look into the feedback? If not, I can just pull in the parts we're comfortable with now, and you could submit the other changes later.

Member

Haacked commented Jul 10, 2012

Ok, I created a new PR that cherry picks the changes we want. #545

Haacked closed this Jul 10, 2012

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