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

Mapbox #958

Merged
merged 85 commits into from
Jan 15, 2018
Merged

Mapbox #958

merged 85 commits into from
Jan 15, 2018

Conversation

corradio
Copy link
Member

@corradio corradio commented Dec 29, 2017

List of changes:

Testable at https://staging.electricitymap.org

List of devices tested and associated performance improvements (please add yours):
(I use ✅ to indicate it works, and 🐇 to indicate it's fast)

  • iPad (non-retina) ✅ 🐇 (major improvement)
  • iPad 3rd gen (retina) ✅ 🐇 (major improvement)
  • iPhone 5S ✅ 🐇 (noticeable improvement)
  • Nexus 6P ✅ 🐇 (panning is much smoother)
  • Nexus 5X ✅ 🐇
  • Nexus 4 ✅ 🐇 (looks better)
  • Thinkpad T430 ✅ 🐇
  • HTC One M9 ✅ 🐇
  • Samsung S7 + chrome : ✅ 🐇
  • Samsung Galaxy Tab S + chrome: ✅ Drag 🐇, Wind 🐢

List of desktop navigators tested on

  • Chrome MacOS ✅ 🐇
  • Chrome PC ?
  • Safari (major improvement) ✅ 🐇
  • Firefox MacOS (major improvement) ✅ 🐇
  • Firefox PC ?
  • Firefox Mobile ✅ 🐇
  • Firefox Linux: ✅ 🐇 (noticeable improvement)
  • IE, Edge ✅

List of remaining tasks:

  • Left panel exchange arrow missing
  • Zone colouring during mouseover is (very) slow
  • Double check loading indicators
  • Fix map drawing bugs
  • Race condition when loading on mobile
  • Pinch zoom on iPhone 5S does not trigger events
  • Rotating is not disabled on mobile
  • On Safari mobile old ipad, after country click, header shouldn't resize
  • Hide arrows before map loads
  • Compare perfs (speed + size of assets) of wind + GIF vs wind + CSS arrows
  • When loading on mobile, half the screen is draggable/drawn
  • Reduce size of arrows
  • Wind layer resize bug on mobile landscape (duplicate of Redraw wind/solar on resize #357)
  • Blurry exchange arrows on zoom on Safari mobile
  • Arrows sizes are not similar
  • Arrows should not change opacity when hovered
  • Firefox/Chrome mobile layout bug
  • Webpack bug:
    image
  • Twitter error:
    image
  • Tooltip error (fix Tapping on a region makes the hover tooltip appear over the detail view #965):
    image
  • on old devices without webgl, the map should be hidden instead of throwing Failed to initialize WebGL

Copy link
Collaborator

@systemcatch systemcatch left a comment

Choose a reason for hiding this comment

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

Big improvement overall to performance/usability imo, looks good to go.

@brunolajoie
Copy link
Contributor

Hey @scarlac ! We need your front end performance expertise here :)

I see a deterioration of the graphical performance due to aliasing on my 22 inch, medium-res screen that no one else seems to see on higher ppi screens: have a look & try it yourself:

  • staging.electricitymap.org
    image

  • electricitymap.org
    image

Alising is visible on country and arrows, but is especially visible on arrows. Would you happen to have an idea why alising appears on arrows? We did not change anything specifically about arrows in this PR, it's still the same gif! Yet they now appear aliased

@scarlac
Copy link
Contributor

scarlac commented Jan 5, 2018

Hey @brunolajoie ! Yeah I see they are pixelated. Testing with Firefox 57 (aka Quantum) in "Low Res" mode on macOS, the fix seems to be to apply translateZ(.01px) to the transform.

Take a look here:
https://www.useloom.com/share/adfeae95093d487b97f6a7dc45ee7b0f

@scarlac
Copy link
Contributor

scarlac commented Jan 5, 2018

I should add that I see the same issue on the production site.

@jarek
Copy link
Collaborator

jarek commented Jan 5, 2018

I'm seeing a bug with top bar on Firefox 57 mobile/Android, see screenshots

screenshot_20180105-221530

screenshot_20180105-221545

@corradio
Copy link
Member Author

corradio commented Jan 6, 2018

@brunolajoie can you try the fix @scarlac mentions and post two screenshots to see if it helps?
@jarek I'll take a look at the bug.

By the way: thanks very much @scarlac !

@systemcatch
Copy link
Collaborator

One small point, on the production map double click lets you zoom in. On staging this doesn't happen though it looks like the arrows refresh.

@corradio
Copy link
Member Author

corradio commented Jan 7, 2018

@systemcatch I disabled double tap zoom because I was having a bug with it.
Do you think it's important?

@systemcatch
Copy link
Collaborator

@corradio It lets people zoom in/out easier than using the buttons on the top right, but I don't think it's that critical tbh.

@brunolajoie
Copy link
Contributor

@scarlac interestingly, the aliasing issue happens both on staging and production in firefox. Yet in chrome, aliasing only appears on staging, and the fix you suggested does not improve it.

@scarlac
Copy link
Contributor

scarlac commented Jan 9, 2018

bruno, Seems very odd but I was using retina but running in Low resolution mode which should be identical from a functional perspective.

Did you try increasing translateZ? The other solutions would be to move around scaling and transforming in hopes it’s an issue related to order of transforms. Perhaps moving transform strait to arrow image or parent. I don’t have a specific solution in mind though.

@corradio
Copy link
Member Author

@jarek bug fixed (I hope)

@jarek
Copy link
Collaborator

jarek commented Jan 10, 2018

Yep that bug seems fixed

@corradio
Copy link
Member Author

..and the last update: we revert to a simpler ui if webgl is not available

image

@corradio corradio merged commit bd7f971 into master Jan 15, 2018
@corradio corradio deleted the olc/mapbox branch January 15, 2018 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants