handleClick(), root state initialization, setting up event listeners, onpopstate(), Drupal Ajax object creation … they would all benefit from being grouped together into a Controller object.

(I realized this thanks to #2713549-4: Refactor onhashchange event handler away: simplicity, consistency, and avoids a whole class of potential problems with JS using hash-based routing.)

CommentFileSizeAuthor
#7 interdiff.txt807 bytesWim Leers
#7 2713567-7.patch27.22 KBWim Leers
#6 interdiff.txt4.89 KBWim Leers
#6 2713567-6.patch27.21 KBWim Leers
#4 interdiff.txt24 KBWim Leers
#4 2713567-4.patch26.03 KBWim Leers
#3 2713567-3.patch4.6 KBWim Leers
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Wim Leers created an issue. See original summary.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Title: Refactor all the controller logic into an actual object » Refactor all the controller logic into an actual object: improve understandability/maintainability
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Active » Needs review
FileSize
2713567-3.patch4.6 KB

First step.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
FileSize
2713567-4.patch26.03 KB
interdiff.txt24 KB

Split out into:

  • HistoryBasedNavigation
  • ClickBasedNavigation
  • Controller

There is unfortunately one regression: when using the "back" button, the scroll position is no longer being restored… deity knows why :(

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented

There is unfortunately one regression: when using the "back" button, the scroll position is no longer being restored… deity knows why :(

Turns out this regression was introduced in #2713549: Refactor onhashchange event handler away: simplicity, consistency, and avoids a whole class of potential problems with JS using hash-based routing! So it'll be up to that issue to fix it.

Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
FileSize
2713567-6.patch27.21 KB
interdiff.txt4.89 KB

Changes:

  • Document architecture
  • s/HistoryBasedNavigation/HistoryNavigation/
  • s/ClickBasedNavigation/LinkNavigation/
  • Explicitly document that we should get rid of the currentPos global (but not in this issue)
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
FileSize
2713567-7.patch27.22 KB
interdiff.txt807 bytes

Fix eslint errors.

  • Wim Leers committed d2152a7 on 8.x-1.x 
    Issue #2713567 by Wim Leers: Refactor all the controller logic into an...
Wim Leers’s picture

Wim Leers
Location Ghent 🇧🇪🇪🇺
CreditAttribution: Wim Leers at Acquia commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.