Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Activity Idea: Planets Activity #711

Merged
merged 52 commits into from
Jun 25, 2020
Merged

New Activity Idea: Planets Activity #711

merged 52 commits into from
Jun 25, 2020

Conversation

AndreaGon
Copy link
Contributor

@AndreaGon AndreaGon commented Mar 18, 2020

I've created a new possible activity for Sugarizer. Planets Activity ver. 1 offers a 3D model of the solar system planets, which allows students to take a closer look at them. This would hopefully help them appreciate and learn more about the Solar System's planets. As always, let me know what you think!

planet1

  • Add Basic functionality
  • Include Fullscreen Mode
  • Allow Responsiveness
  • Add name of each planet on home view
  • Select/Unselect Rotation & Info icons
  • Replace "Go Home" button with "Back" button
  • Add relative position/size of planets
  • Handle Journal
  • Include Tutorial
  • Add localization

This includes the images, icons, and css
showInfo requires double clicking to work; Now it only requires one time click
@llaske
Copy link
Owner

llaske commented Mar 18, 2020

Very cool idea and very good start 👍

Do you use a portable 3D library? Mean that could work on Chrome/Firefox/Safari/Android/iOS?
Don't forget to credit the source of image/content.

An UI suggestion: may be you could use a black background on the home page.

Plan a smartphone mode too (fullscreen button in the toolbar), it's now a pre-requisite for new activities.

Can't wait to see the final result.

@AndreaGon
Copy link
Contributor Author

I used Three.js library, and base on my research, I think it's portable.

In this latest commit, I made the activity more responsive and I've also added a fullscreen mode. The home page's background should also be black (it is much nicer).

@llaske
Copy link
Owner

llaske commented Mar 19, 2020

Nice.

Few suggestion:

  • Will be nice to add the name of each planet on the home view
  • Will be nice to have a way to see relative position/size of planets in the solar system. Could be another view or a small graphic in the detailed view of each planet
  • Rotation and Info icons should be selected/unselected depending of their state
  • Not a big fan of the "Go home" button and of the house icon. May be could it be replaced by a planet list to remind the home view or removed of the toolbar and replaced by a back icon into the main view (like in Calligra activity)
  • A set of error appears in the console on my side (look like some file are missing):

image

This should fix the error in the console. Also, I've added the back button functionality and included planet names in the planet list. The rotation & info button should now able to be selected/unselected.
There's a bug where the planet rotation renders the previous animation on top of one another, thus giving the illusion where the rotation goes faster and faster. I've added a fix where the rotation animation is cancelled when back button was clicked.
@AndreaGon
Copy link
Contributor Author

@llaske I've fixed the console error and added the the suggested enhancement.

I have a question regarding adding the relative position/size of planets. Did you mean something like this:

position

@llaske
Copy link
Owner

llaske commented Mar 28, 2020

@AndreaGon yes, exactly.

BTW, look like some files are missing in your last commit.

image

@AndreaGon
Copy link
Contributor Author

I've added the planet position and size in this latest commit. This should also fix the error message on the console.

@llaske
Copy link
Owner

llaske commented Apr 11, 2020

Very nice.

Few remarks:

  • The compare-icon.svg is missing in the repo, so the button is not visible
  • It will be nice to display the sun (partially) in the new view, else we don't know if the sun is to the left or to the right of the screen :-)
  • The image for Saturn on planet position is weird, we expect a view where rings is over the planet
  • The back to planets lists button should be visible even when planet info are not displayed
  • May be you could add the Distance to sun value in the planet info too

@AndreaGon
Copy link
Contributor Author

I've included the tutorial and localization. The back button should be fixed as well :)

@llaske
Copy link
Owner

llaske commented May 19, 2020

Nice. The Back button is now here.

Few remarks:

  • There is large black margin at the left of planets list, it was not there before. Is it a consequence of the Back button?

image

  • The distance to the sun in the planet distance screen is so big that they overlap each other. I wonder if it's a good idea to display it here. May be we could put it as property of the planet detail screen.

image

  • Why not allow the user to click on the planets in the distance screen to display directly the detail screen for this planet (may be we could detect a rectangle around smaller planets)?

When user clicks the planets in planet pos view, the explore view should appear
The rectangles (div tags) serve as hit boxes for smaller planets
@AndreaGon
Copy link
Contributor Author

I've added the enhancements in these commits.

The margin on the left is also fixed. It wasn't caused by the back button, but it's because I tried putting the list view in the middle.

Let me know what you think!

@llaske
Copy link
Owner

llaske commented Jun 4, 2020

Very nice. It's cool like that.

Just an issue thought: in touch mode, the planet position screen doesn't work. When touching planets, nothing happens.

image

Plus if possible avoid scrollbar in this screen.

@AndreaGon
Copy link
Contributor Author

This should fix the issue. When switching to touchscreen mode, the browser needs to be refreshed for it to detect whether it is in touchscreen mode or not.

Thoughts?

@llaske
Copy link
Owner

llaske commented Jun 13, 2020

Work very nice now.

Just found another small issue (not related to touch):

  • Create a new instance of the activity
  • Click on a planet to see detail
  • Click on toggle info button in the toolbar to hide info
  • Go back to the list of planet
  • Click on a planet again
  • The info is displayed but the toolbar button is unselected so to hide the info you should click two times on the button

The same thing also happens to rotation button. This should fix it
@AndreaGon
Copy link
Contributor Author

I've fixed the issue in this commit. The same thing happens to rotation button, so I also fixed it.

@llaske
Copy link
Owner

llaske commented Jun 14, 2020

Good.
There is an issue in localization, planets name are not localized in list view.

image

I've tried to localize it here but it don't works because it seems that the localized event is not yet raised.

@AndreaGon
Copy link
Contributor Author

The localization issue should now be fixed. I also did the same thing for the planet position view.

@llaske
Copy link
Owner

llaske commented Jun 16, 2020

The problem now is that the lists is filled two times.

Peek 16-06-2020 22-58

It happens because the localized event is raised two times by the l10n framework. The first time for the browser language, the second time for the Sugarlizer language. May be you could remove the content of the list before fill it.

@llaske
Copy link
Owner

llaske commented Jun 17, 2020

Just seen another issue on Safari (MacOS) and on iOS. The planet name in the list view is not displayed under the planet image.

image

May be you're using a display style not supported on some browsers.

@AndreaGon
Copy link
Contributor Author

The extra list view should now be removed. As for the list view in Safari, I made some changes on the CSS file as well. Let me know if the issue still persist.

@llaske llaske merged commit 8edd288 into llaske:dev Jun 25, 2020
@llaske
Copy link
Owner

llaske commented Jun 25, 2020

Works very well now on all platforms.

Thanks a lot to add this beautiful activity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants