oppia / foundation-website

Repository for developing the new Oppia Foundation website.
Apache License 2.0
6 stars 33 forks source link

Fix #64: Remove ng-view autoscroll config to preserve previous scroll location #74

Closed hoangviet1993 closed 6 years ago

hoangviet1993 commented 6 years ago

Fix #64 (I hope this is the fix, otherwise a prolonged PR is expected) I think the test server should have the behavior that you are looking for @seanlip

  1. Go to home page. Scroll to a certain location.
  2. Go to partnerships page.
  3. Click back button. The browser view should remain at step 1 location.

Please lmk if there is something I am overlooking.

Changes are live at: https://oppia-foundation-test-server.appspot.com/

codecov-io commented 6 years ago

Codecov Report

Merging #74 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #74   +/-   ##
=======================================
  Coverage   95.94%   95.94%           
=======================================
  Files          18       18           
  Lines         345      345           
=======================================
  Hits          331      331           
  Misses         14       14

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ce12154...c057e1b. Read the comment docs.

seanlip commented 6 years ago

Ah, I don't think this is the fix I'm afraid. If you click the Partnerships page (step 2) then you start off in the middle of the page and not the top, as expected.

hoangviet1993 commented 6 years ago

@seanlip Ah got it. So there is a catch after all!

How about now? https://oppia-foundation-test-server.appspot.com/

seanlip commented 6 years ago

Still another catch :)

Try scrolling and navigating just by clicking buttons in the navbar. You'll find that, the second time you visit the homepage (all just through forward clicks) it will "remember" the scroll position of the previous time you visited it.

It shouldn't; the only time that that should happen is when you use the Back button (in parity with behaviour elsewhere on the Internet).

hoangviet1993 commented 6 years ago

@seanlip can you please be specific with your reproduction steps?

I am having difficulty to replicate it with the following behavior:

  1. Navigate to home page using provided Url. Landed at hero section.
  2. Navigate to partnerships page using navbar and scroll down to "evidence of our success".
  3. Navigate to home page. Expect to be at hero section again.
  4. Navigate to partnerships page using navbar. Landed at "evidence of our success" section.

(Can you clear browser temp files and stuff like that before hand)?

seanlip commented 6 years ago

Ah, sorry! Yes of course.

  1. On large screen, navigate to home page and scroll down.
  2. Click on Partnerships page in navbar and scroll down.
  3. Click on top-left icon. Expect to be at top of home page but it seems to be in the middle.
  4. Click on Partnerships page in navbar. Expect to be at top of partnerships page but it seems to land in the middle.

(Landing in the middle should only happen when Back button is clicked, but not on forward navigation. Also I confirm that I've cleared my cache.)

seanlip commented 6 years ago

In other words, navigating via navbar, or by typing in URL, should always land at top of page. It's only when clicking back that the scroll position should be remembered.

hoangviet1993 commented 6 years ago

@seanlip

Thank you so much for clarifying these details. I never really paid attention to these minute details and took them for granted as a user.

I have tested out these scenarios: Scenario 1:

  1. Navigate to home page and scroll down to "Partner with us".
  2. Navigate to Partnerships page using navbar. Verify that page view is at top of page.
  3. Click "Back" button. Page view is now back at home page's "Partner with us"

Scenario 2:

  1. Navigate to home page and scroll down to "Partner with us".
  2. Navigate to Partnerships page using using navbar. Verify that page view is at top of page.
  3. Click Oppia foundation logo. Verify that page view is at top of page.

Scenario 3,4 (replicate the above scenarios using Url)

Can you PTAL again?

Link:https://oppia-foundation-test-server.appspot.com/

hoangviet1993 commented 6 years ago

@seanlip I am not sure tbh. I think I can justify needing $window.pageXOffset when the page is scroll-able horizontally. And I think if I try (really hard), I can get the page to be scroll-able sideway.

I don't really mind get rid of it tbh. What do you think should be the call here?

seanlip commented 6 years ago

Eh, either's fine I think :) Let's just merge it! Thanks!