883192 - thimble "make details" living on the page instead of an iframe causes page CSS to override make detail styling

Status: RESOLVED FIXED

(Webmaker Graveyard :: Thimble, defect)

Description

[Pomax]

The only real fix is to sandbox, and have the makedetails live as an iframe on the made page for Thimble. This is a big change, and I'll leave it up to scott to determine whether this is doable today. If not, it's a fast-fast-followup.

Comment 3

[Matthew Schranz [:mjschranz]]

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

We already moved to this kind of deployment in Popcorn Maker. This is a patch I was working on for getting this to work in Thimble.

Comment 4

[Scott [:thecount] Downe]

Attachment: error

Hmm, not working for me.

Comment 5

[Scott [:thecount] Downe]

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

I added a screenshot of what I got when I tried to use this.

Also, the made with and remix buttons should probably be responsive and stick to the left and right sides. If I shrink my window, they should stay in view.

Comment 6

[Pomax]

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

This does not solve the problem that the ticket is for. There's two problems:

- make detail CSS affecting the page
- page CSS affecting the make detail HTML

we already solved (1) by namespacing, so that part's good, but we can't solve (2) without isolating the make details in an iframe.

Comment 7

[Scott [:thecount] Downe]

OK, I got the page to display.

I still have issues with the css of the details bar.

Also, the css in the actual details page looks off.

Attaching a screen shot of what I get now.

Comment 8

[Scott [:thecount] Downe]

Attachment: not enough space to display make

Comment 9

[Matthew Schranz [:mjschranz]]

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

NEw PR. Most code is the same but I did add in one namespace fix.

Comment 10

[Pomax]

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

this puts the details themselves in an iframe, which is a good start, but the make details bar is still "on page", so that still gets messed with quite spectacularly (i,e, http://s3.amazonaws.com/org.webmadecontent.staging.thimble/pomax/thimble/vidyo-conan-hack)

Comment 11

[Scott [:thecount] Downe]

Just add:

 .wrapper {
   position: relative;
-  width: 1020px;
+  width: 100%;
   margin: 0 auto;
 } 

to the details.css file to clear up my final nit. (the maring 50px can be done in another ticket beause it is webmaker.org, correct?)

Comment 12

[Scott [:thecount] Downe]

Also, ensure the /templates/tutorial path still disabled any css needed to disable the details bar.

Comment 13

[Scott [:thecount] Downe]

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

R+ when you have a ticket to sort out the width of the bar, and the tutorial template does not show the details bar,

Comment 14

[Matthew Schranz [:mjschranz]]

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

Updated.

Comment 15

[Scott [:thecount] Downe]

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

Comment in the ticket.

Comment 16

[Pomax]

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

I'm okay with this, as long as the "mobile" mystery element is either explained or removed =)

Comment 17

[[github robot]]

Commit pushed to master at https://github.com/mozilla/thimble.webmaker.org

https://github.com/mozilla/thimble.webmaker.org/commit/7d6b19b21a6d18ba554205b00d2f071ff95c75a9
Fix Bug 883192 - Change details page layout for new make details