Skip to content
This repository has been archived by the owner on Jul 31, 2019. It is now read-only.

[WIP] Fix issue 2012: Make remix button working #2027

Closed
wants to merge 1 commit into from

Conversation

ZenanZha
Copy link
Contributor

This is a possible sulotion for issue2012.
Need more discussion on how to fix it, so I kept the old code.

Merge this would temporarily make remix button working again.

@gideonthomas @humphd

@@ -188,12 +188,15 @@ define(["jquery", "analytics"], function($, analytics) {
newItem.find(".description").text(activity.description);

// Check if activity_url ends with a slash, if it doesn't - add one before adding "remix"
var remix = "remix";
/*var remix = "remix";
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to comment out all of this, just remove it.

var endsWithSlash = (activity.url.charAt(activity.url.length-1) === "/");
if(!endsWithSlash) {
remix = "/remix";
}
newItem.find(".remix").attr("href", activity.url + remix);
*/
//WIP: see the discussion on: https://github.com/mozilla/thimble.mozilla.org/issues/2012
newItem.find(".remix").attr("href", "https://thimble.mozilla.org/projects/" + (activity.url).match(/\d+/)[0] + "/remix");
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm going to suggest another fix here because regexing urls can be brittle.
@ZenanZha open up a PR against the https://github.com/mozilla/thimble-homepage-gallery repo and add a property to each object called projectID and let its value be the number at the end of the url property. FOr e.g.,

{
  "title" : "Back to School Postcard",
  "thumbnail_url" : "https://thimble.mozilla.org/img/thumbnail-postcard.png",
  "url" : "https://thimbleprojects.org/mozillalearning/220079/",
  "description" : "Add your own content to this interactive postcard.",
  "tags" : ["images","code","javascript","interactive","js"],
  "teaching_kit_url" : "https://thimbleprojects.org/mozillalearning/62/",
  "featured" : true,
  "author" : "mozilla",
  "author_url" : "http://learning.mozilla.org"
}

would change to

{
  "title" : "Back to School Postcard",
  "thumbnail_url" : "https://thimble.mozilla.org/img/thumbnail-postcard.png",
  "url" : "https://thimbleprojects.org/mozillalearning/220079/",
  "projectID" : 220079,
  "description" : "Add your own content to this interactive postcard.",
  "tags" : ["images","code","javascript","interactive","js"],
  "teaching_kit_url" : "https://thimbleprojects.org/mozillalearning/62/",
  "featured" : true,
  "author" : "mozilla",
  "author_url" : "http://learning.mozilla.org"
}

Then, out here, change this line to:

newItem.find(".remix").attr("href", "/projects/" + activity.projectID + "/remix");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gideonthomas
But what is the point for us to keep "url" in the file?
We didn't use it anywhere.

Copy link
Contributor

Choose a reason for hiding this comment

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

because it stores the username part of the url which is needed.

@gideonthomas
Copy link
Contributor

@ZenanZha any updates on this?

@Rajat-dhyani
Copy link
Contributor

@ZenanZha still working on this? Mind if i take over ?

@gideonthomas
Copy link
Contributor

Hi @Rajat-dhyani and @ZenanZha, this has become a critical bug that needs to be fixed so I think I'm gonna take over so that we can land this asap. Thank you for getting this started though!

@gideonthomas gideonthomas self-assigned this May 8, 2017
Rajat-dhyani added a commit to Rajat-dhyani/thimble-homepage-gallery that referenced this pull request May 8, 2017
Rajat-dhyani added a commit to Rajat-dhyani/thimble-homepage-gallery that referenced this pull request May 8, 2017
@Rajat-dhyani
Copy link
Contributor

Rajat-dhyani commented May 8, 2017

No problem @gideonthomas Thanks for informing i was already working on this 👍 i will skip it

@gideonthomas gideonthomas mentioned this pull request May 10, 2017
@gideonthomas
Copy link
Contributor

Moved to #2051!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants