search

18F / epa-notice

Web interface for viewing and commenting on proposed changes to federal regulations
Other
7 stars 17 forks source link

"Edit" comments doesn't change scroll position #158

Open cmc333333 opened 8 years ago

cmc333333 commented 8 years ago

1) Add a comment to Section III.B 2) Add a comment to Section III.C.1 3) Navigate to write mode of a different section 4) Click "edit" for the III.B comment. Notice that the url changes to preamble/2016_02749/III#2016_02749-III-B 5) Click "edit" for the III.C.1 comment. Note that the url does not change

Reversing steps 5 and 4 results in reversed urls.

jmcarp commented 8 years ago

Took a look at this--it looks like backbone has opinions about routing that don't match how we're expecting it to work. Specifically, Router.navigate bails out if the path of the current url is the same as the path of the url to open, even if the hash differs--so if we're at /preamble/2016_02749/I#2016_02749-I-A and want to navigate to /preamble/2016_02749/I#2016_02749-I-B, Router thinks we don't need to change history. It turns out that lots of people have complained about this behavior, but it's not going to change.

One simple but awful thing we could do is to override navigate, paste in the contents of Backbone.Router.navigate, and delete the offending line. I'll look for a better approach, but in the meantime, @cmc333333 @xtine: let me know what you think about this.

jmcarp commented 8 years ago

For example, this works, as far as I can tell, but it's upsetting: https://github.com/eregs/regulations-site/compare/master...jmcarp:respect-hash?expand=1. What do you all think?

cmc333333 commented 8 years ago

Seems like there should already be an easier solution. How does this work when there's an internal link on the same page? For example

  1. https://atf-eregs.apps.cloud.gov/447-11/2015-12992#447-11-p3984137023
  2. Click "articles" in this paragraph
  3. Click "§ 447.11" in the rhs
  4. URL adjusts as does the viewport
jmcarp commented 8 years ago

I'll verify this tomorrow, but it looks like we're hitting the scroll callback there (the openFullDefinition callback in DefinitionView opens a new section and passes a scroll-to hash, which triggers the scroll callback). The scroll callback manually calls replaceState and skips the backbone router, so that we don't add a new entry to the browser history. I would guess we can take the same approach here (passing a scroll-to hash when we edit an existing comment), as long as we don't expect to get a new history entry. If a user edits a few comments in a row and clicks back, what do we expect to happen?