871558 - Display tutorials in thimble

Status: RESOLVED FIXED

(Webmaker Graveyard :: Thimble, defect)

Description

[Scott [:thecount] Downe]

Same thing we've done in popcorn maker (bug 866923) for thimble.

You can create a thimble project that is a tutorial and have it display in thimble, from the webmaker nav.

A tutorial is defined by a meta tag <meta name="webmaker:tutorial" content="url to project">

This links it to the project. Has some UI, and displays a list of steps that are used as a tutorial.

Comment 1

[Scott [:thecount] Downe]

Attachment: https://github.com/mozilla/thimble.webmaker.org/pull/57

Comment 2

[Pomax]

Comment on attachment 748977 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/57

There's some nits, but also an approach comment; there's a lot of "pure JS" in the page code, with jQuery used alongside. However, jQuery will make a lot of the code more concise, so I'd recommend that if we're loading jQuery to begin with, tighten up this code so that we're not using document.querySelector and [...].addEventListener('click',...), but rather use $("...") and $(...).click etc.

Comment 3

[Scott [:thecount] Downe]

Comment on attachment 748977 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/57

Fixed or commented on everything.

Comment 4

[Pomax]

Comment on attachment 748977 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/57

few more nits, mostly involving jQuery

Comment 5

[Scott [:thecount] Downe]

Comment on attachment 748977 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/57

Yeah, I was hesitant on the jquery in some spots. Updated it, should all be jquery now.

I also found two issues.

There was some less that snuck in.

If you closed the window after resize happened, jquery ui's width css specificity would override my class, thus the container would not actually be closed.

Comment 6

[Pomax]

Comment on attachment 748977 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/57

looks good. One nit in the pull request, but doesn't need another review pass after fixing. Land ho~

Comment 7

[Cassie McDaniel [:cassiemc]]

Attachment: Tutorial UI

Adding a design for the tutorial popup view from bug 862361. 

Scott, I've tried to keep it functionally the same, but let me know if you foresee any issues implementing this. I've also just shared the PSD with you from Drive.

Comment 8

[Scott [:thecount] Downe]

Do we need a remix for the tutorial? Only people with certain permissions can make or remix tutorials anyway. Thimble had a remix this ribbon, but the intent was to remove it.

Looks like there needs to be some images, though, for the resize handle to get that angle.

Same for the tutorial button, it needs an image.

I am worried that technically, doing the resize handle might be problematic, as it uses jquery UI but it is more than worth a shot.

Going to land it as is, and move the UI updates into a new ticket? Sound good?

Comment 9

[Scott [:thecount] Downe]

Comment on attachment 748977 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/57

Rebased and fixed up.

Going to throw this back up for one more review as there were some interesting rebases, and just need to make sure everything is sane.

Comment 10

[Pomax]

Comment on attachment 748977 [details] [review]
https://github.com/mozilla/thimble.webmaker.org/pull/57

we need to start refactoring index.html so that all the JS lives in actual .js files that we include, instead of sitting the same file and generating a huge bloated single file. Can you file a follow-up bug for the refactor job, and then we can land this one?

Comment 11

[Scott [:thecount] Downe]

Bug 873344 is to move index.html into smaller js files.

Comment 12

[Scott [:thecount] Downe]

Staged: https://github.com/mozilla/thimble.webmaker.org/commit/92d2c70408a3b51f3f8db2585fe3723d024e47b2

Comment 13

[Cassie McDaniel [:cassiemc]]

(In reply to Scott [:thecount] Downe from comment #8)
> Do we need a remix for the tutorial? Only people with certain permissions
> can make or remix tutorials anyway. Thimble had a remix this ribbon, but the
> intent was to remove it.

I do think we need it. It would link to the tutorial thimble detail page. Can it only show if users are logged in and able to remix it? Is the thought process for removing the ribbon documented anywhere, so I can catch up?


> Looks like there needs to be some images, though, for the resize handle to
> get that angle.
> 
> Same for the tutorial button, it needs an image.
> 
> I am worried that technically, doing the resize handle might be problematic,
> as it uses jquery UI but it is more than worth a shot.

Just use a placeholder for now, if you need the graphics I can help cut them.

> Going to land it as is, and move the UI updates into a new ticket? Sound
> good?

Did you make a new ticket for this?