councilforeconed / interactive-activities

Council for Economic Education
http://interactives.councilforeconed.org
Mozilla Public License 2.0
6 stars 2 forks source link

Pizza workstation #119

Closed ZeeJab closed 9 years ago

ZeeJab commented 10 years ago

@jugglinmike this branch has the images for the pizza workstation that you need

jugglinmike commented 10 years ago

@ZeeJab I've implemented the functionality, but there's a small problem: according to the current progression of workstations (dough -> sauce -> cheese -> anchovies -> olives), the pizza will never be rendered with olives directly. Instead, in the final station, it will be rendered with anchovies, and the player will place olives on the pizza themselves. This means we need an image of a pizza with anchovies but without olives.

In the mean time, I've re-named your olives image to anchovies. The progression looks a little strange, but now you can simply save a new version of the image, and everything should fall into place.

By the way, I noticed that a few images specific to the "Pizza" activity are in the src/client/images/ directory. To keep things organized, I recommend placing activity-specific content inside each activity's sub-directory in src/activities/.

ZeeJab commented 10 years ago

@jugglinmike I think they had planned on removing anchovies all together... but the way you've implemented it should do the job for now... /cc @stevekinney

stevekinney commented 10 years ago

Yea, we were going to cut the anchovies. :fish:

stevekinney commented 9 years ago

Hey, @jugglinmike and @ZeeJab! I've been out of town for the last few days and I'm trying to catch up with life. Does anyone need anything from me with this (alternatively, is there something I can do that's not going to step on someone's toes)?

ZeeJab commented 9 years ago

Hi @stevekinney! :-) I believe the only thing would be to take out anchovies from the workflow and then test out to see if everything is/looks how it should?

On Monday, July 14, 2014, Steve Kinney notifications@github.com wrote:

Hey, @jugglinmike https://github.com/jugglinmike and @ZeeJab https://github.com/ZeeJab! I've been out of town for the last few days and I'm trying to catch up with life. Does anyone need anything from me with this (alternatively, is there something I can do that's not going to step on someone's toes)?

— Reply to this email directly or view it on GitHub https://github.com/councilforeconed/cee/pull/119#issuecomment-48907963.

Sent by Carrier Pigeon

stevekinney commented 9 years ago

Alright cool, I just didn't want to be that guy who comes back from "vacation" and then gets in the way!

jugglinmike commented 9 years ago

Is removing anchovies something you guys would like me to look into?

stevekinney commented 9 years ago

That would be amazing if you could!

jugglinmike commented 9 years ago

Alrighty. I expect to have some time for this tomorrow evening. Will that be okay?

stevekinney commented 9 years ago

Perfect. Thanks so much!

jugglinmike commented 9 years ago

Alrighty @ZeeJab I think that does it for the Anchovies. Would you mind playing through and verifying:

  1. The images are all correct
  2. That the sequencing functions as expected (with no bugs or crashes). Specifically, here's what you can expect:
    • The workstations and pizza "state" should go: dough, sauce, cheese, and olives.
    • After returning the olive'd pizza to the queue, it should be automatically removed from the queue and the player's total pizza count should increment.

This all seems correct on my machine. Let me know if anything seems missing/wrong on your end!

ZeeJab commented 9 years ago

On it! :rabbit:

ZeeJab commented 9 years ago

This looks correct to me, everything behaves as I understand it should. If there are no objections, I'll merge?

stevekinney commented 9 years ago

DO IT!

Sent from my iPhone

On Jul 18, 2014, at 5:41 PM, Zahra Jabini notifications@github.com wrote:

This looks correct to me, everything behaves as I understand it should. If there are no objections, I'll merge?

— Reply to this email directly or view it on GitHub.

stevekinney commented 9 years ago

So, I was writing from my phone. I got a chance to kick the tires a little harder this morning. It looks amazing, but there are few little edges left.

I'm more concerned with the first issue than the second.

ZeeJab commented 9 years ago

@stevekinney ...so I messed around with the code [the code I think is associated with this] for a few hours yesterday and I can't figure out where this is supposed to happen or how I can change what is happening? Help... -.o

stevekinney commented 9 years ago

With the multiple pizzas on the conveyor belt or with changing the status on the pizza as soon as the final #{ingredient} is put on?

stevekinney commented 9 years ago

Hmm… maybe @jugglinmike has some guidance?

jugglinmike commented 9 years ago

This is a CSS bug. The markup is being rendered as expected (including the data- attribute we discussed previously):

screenshot from 2014-07-30 20 17 20

This commit removed the width declaration from the .pizza-queue-pizza-container element, and as a result, all of the pizza icons are now rendered on top of each other. An explicit width is unfortunate-but-necessary here because the content (the pizza element itself) is set to position: absolute to support dragging.

A simple fix would be to re-declare the width, but a more maintainable solution might be possible if the pizza elements were styled with position: relative instead. It's been a while since I looked at this code (and the layout has changed significantly since then), so I'm not sure if this will be possible, but it's worth looking in to.

stevekinney commented 9 years ago

You are the best.

On Jul 30, 2014, at 8:42 PM, jugglinmike notifications@github.com wrote:

This is a CSS bug. The markup is being rendered as expected (including the data- attribute we discussed previously):

This commit removed the width declaration from the .pizza-queue-pizza-container element, and as a result, all of the pizza icons are now rendered on top of each other. An explicit width is unfortunate-but-necessary here because the content (the pizza element itself) is set to position: absolute to support dragging.

A simple fix would be to re-declare the width, but a more maintainable solution might be possible if the pizza elements were styled with position: relative instead. It's been a while since I looked at this code (and the layout has changed significantly since then), so I'm not sure if this will be possible, but it's worth looking in to.

— Reply to this email directly or view it on GitHub.

ZeeJab commented 9 years ago

o.O woah. Nice. Thank you!! Fixing.

Z! Don't be great. Be Magical http://www.zahraism.com www.manhattanjs.com

On Wed, Jul 30, 2014 at 8:43 PM, Steve Kinney notifications@github.com wrote:

You are the best.

On Jul 30, 2014, at 8:42 PM, jugglinmike notifications@github.com wrote:

This is a CSS bug. The markup is being rendered as expected (including the data- attribute we discussed previously):

This commit removed the width declaration from the .pizza-queue-pizza-container element, and as a result, all of the pizza icons are now rendered on top of each other. An explicit width is unfortunate-but-necessary here because the content (the pizza element itself) is set to position: absolute to support dragging.

A simple fix would be to re-declare the width, but a more maintainable solution might be possible if the pizza elements were styled with position: relative instead. It's been a while since I looked at this code (and the layout has changed significantly since then), so I'm not sure if this will be possible, but it's worth looking in to.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub https://github.com/councilforeconed/cee/pull/119#issuecomment-50700207.