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

Proposal: remove widget code #1224

Closed
chrisrosset opened this issue Jul 2, 2023 · 1 comment · Fixed by #1250
Closed

Proposal: remove widget code #1224

chrisrosset opened this issue Jul 2, 2023 · 1 comment · Fixed by #1250

Comments

@chrisrosset
Copy link
Collaborator

@jpatokal, I would like to propose the removal of the map widget code.

The map widget hasn't seen any meaningful updates since it was first created in Feb 2010 (880b3f3 and 880b3f3). I'm afraid it's beyond redemption - I doubt this code can be reasonably embedded into any modern website.

Removing this code would reduce the changes needed to support transitioning to JSON-based server APIs (which I've already started in #1222 with top10.php) and upgrading OpenLayers (ongoing discussion in #1192 with some work in progress). It would also allow us to remove prototype.js if the autocomplete upgrade (#1223) gets merged.

chrisrosset added a commit to chrisrosset/openflights that referenced this issue Jul 10, 2023
This PR rips out the current autocomplete control with its two big
legacy dependencies:

1. `prototype.js`[^1] (no updates since 2015)
2. `scriptalicious.js` (no updates since 2010)

and replaces them with the much much smaller, modern, and actively
maintained https://github.com/kraaden/autocomplete. We use
denis-taran/autocomplete@589da82
which is versioned as 8.0.4 on NPM (https://www.npmjs.com/package/autocompleter/v/8.0.4).
The end result replicates the functionality almost exactly.
Thanks to @reedy, the CSS matches the existing color palette too!

The reason for this change is that the current autocomplete framework
blocks multiple other improvements. `autocomplete.php` currently returns
an HTML fragment as the response which is then embedded into the DOM.

Request payload: `src_ap=new`

Response payload:

```html
<ul class='autocomplete'><li class='autocomplete' origin='src_ap' id='NEW:9188:-90.028297424316:30.042400360107:-6:A'>New Orleans-Lakefront Airp. (NEW), United States</li>
<li class='autocomplete' origin='src_ap' id='EWB:5737:-70.95690155029297:41.67610168457031:-5:A'>New Bedford Regional Airpo. (EWB), United States</li>
<li class='autocomplete' origin='src_ap' id='EWN:3730:-77.04290008539999:35.0730018616:-5:A'>New Bern-Coastal Carolina. (EWN), United States</li>
<li class='autocomplete' origin='src_ap' id='HVN:4006:-72.88680267:41.26369858:-5:A'>New Haven-Tweed New Haven. (HVN), United States</li>
<li class='autocomplete' origin='src_ap' id='MSY:3861:-90.25800323486328:29.99340057373047:-6:A'>New Orleans-Louis Armstron. (MSY), United States</li>
<li class='autocomplete' origin='src_ap' id='NBG:3768:-90.03500366:29.82530022:-6:A'>New Orleans NAS JRB/Alvin. (NBG), United States</li>
</ul>
```

This is super tight coupling between the UI and the server. This UI
dependency has to be removed before we can upgrade the endpoint.

I use `fetch` to send the request to `autocomplete.php` but then process
the response as data instead of embedding it in the DOM. I parse the
response as XML using `DOM.Parser` (this part will become `JSON.parse()`
once the API starts returning JSON) and pass the data to
`autocomplete.js` which deals with showing the list of items.

[^1]: We still need `prototype.js` in the map widget. See jpatokal#1224.
chrisrosset added a commit to chrisrosset/openflights that referenced this issue Jul 10, 2023
This code hasn't seen any action since it was created in early 2010. It
has suffered major bitrot. The state of the code makes it impossible to
actually integrate into any modern website. The fact that the widget
requires running your own instance of OpenFlights is also a major
dealbreaker.

The demo as seen on the actual website is broken in a number of ways:
https://openflights.org/widget/demo

Closes jpatokal#1224.
reedy pushed a commit that referenced this issue Jul 10, 2023
This PR rips out the current autocomplete control with its two big
legacy dependencies:

1. `prototype.js`[^1] (no updates since 2015)
2. `scriptalicious.js` (no updates since 2010)

and replaces them with the much much smaller, modern, and actively
maintained https://github.com/kraaden/autocomplete. We use
denis-taran/autocomplete@589da82
which is versioned as 8.0.4 on NPM (https://www.npmjs.com/package/autocompleter/v/8.0.4).
The end result replicates the functionality almost exactly.
Thanks to @reedy, the CSS matches the existing color palette too!

The reason for this change is that the current autocomplete framework
blocks multiple other improvements. `autocomplete.php` currently returns
an HTML fragment as the response which is then embedded into the DOM.

Request payload: `src_ap=new`

Response payload:

```html
<ul class='autocomplete'><li class='autocomplete' origin='src_ap' id='NEW:9188:-90.028297424316:30.042400360107:-6:A'>New Orleans-Lakefront Airp. (NEW), United States</li>
<li class='autocomplete' origin='src_ap' id='EWB:5737:-70.95690155029297:41.67610168457031:-5:A'>New Bedford Regional Airpo. (EWB), United States</li>
<li class='autocomplete' origin='src_ap' id='EWN:3730:-77.04290008539999:35.0730018616:-5:A'>New Bern-Coastal Carolina. (EWN), United States</li>
<li class='autocomplete' origin='src_ap' id='HVN:4006:-72.88680267:41.26369858:-5:A'>New Haven-Tweed New Haven. (HVN), United States</li>
<li class='autocomplete' origin='src_ap' id='MSY:3861:-90.25800323486328:29.99340057373047:-6:A'>New Orleans-Louis Armstron. (MSY), United States</li>
<li class='autocomplete' origin='src_ap' id='NBG:3768:-90.03500366:29.82530022:-6:A'>New Orleans NAS JRB/Alvin. (NBG), United States</li>
</ul>
```

This is super tight coupling between the UI and the server. This UI
dependency has to be removed before we can upgrade the endpoint.

I use `fetch` to send the request to `autocomplete.php` but then process
the response as data instead of embedding it in the DOM. I parse the
response as XML using `DOM.Parser` (this part will become `JSON.parse()`
once the API starts returning JSON) and pass the data to
`autocomplete.js` which deals with showing the list of items.

[^1]: We still need `prototype.js` in the map widget. See #1224.
reedy pushed a commit that referenced this issue Jul 10, 2023
This code hasn't seen any action since it was created in early 2010. It
has suffered major bitrot. The state of the code makes it impossible to
actually integrate into any modern website. The fact that the widget
requires running your own instance of OpenFlights is also a major
dealbreaker.

The demo as seen on the actual website is broken in a number of ways:
https://openflights.org/widget/demo

Closes #1224.
reedy pushed a commit to reedy/openflights that referenced this issue Jul 10, 2023
This PR rips out the current autocomplete control with its two big
legacy dependencies:

1. `prototype.js`[^1] (no updates since 2015)
2. `scriptalicious.js` (no updates since 2010)

and replaces them with the much much smaller, modern, and actively
maintained https://github.com/kraaden/autocomplete. We use
denis-taran/autocomplete@589da82
which is versioned as 8.0.4 on NPM (https://www.npmjs.com/package/autocompleter/v/8.0.4).
The end result replicates the functionality almost exactly.
Thanks to @reedy, the CSS matches the existing color palette too!

The reason for this change is that the current autocomplete framework
blocks multiple other improvements. `autocomplete.php` currently returns
an HTML fragment as the response which is then embedded into the DOM.

Request payload: `src_ap=new`

Response payload:

```html
<ul class='autocomplete'><li class='autocomplete' origin='src_ap' id='NEW:9188:-90.028297424316:30.042400360107:-6:A'>New Orleans-Lakefront Airp. (NEW), United States</li>
<li class='autocomplete' origin='src_ap' id='EWB:5737:-70.95690155029297:41.67610168457031:-5:A'>New Bedford Regional Airpo. (EWB), United States</li>
<li class='autocomplete' origin='src_ap' id='EWN:3730:-77.04290008539999:35.0730018616:-5:A'>New Bern-Coastal Carolina. (EWN), United States</li>
<li class='autocomplete' origin='src_ap' id='HVN:4006:-72.88680267:41.26369858:-5:A'>New Haven-Tweed New Haven. (HVN), United States</li>
<li class='autocomplete' origin='src_ap' id='MSY:3861:-90.25800323486328:29.99340057373047:-6:A'>New Orleans-Louis Armstron. (MSY), United States</li>
<li class='autocomplete' origin='src_ap' id='NBG:3768:-90.03500366:29.82530022:-6:A'>New Orleans NAS JRB/Alvin. (NBG), United States</li>
</ul>
```

This is super tight coupling between the UI and the server. This UI
dependency has to be removed before we can upgrade the endpoint.

I use `fetch` to send the request to `autocomplete.php` but then process
the response as data instead of embedding it in the DOM. I parse the
response as XML using `DOM.Parser` (this part will become `JSON.parse()`
once the API starts returning JSON) and pass the data to
`autocomplete.js` which deals with showing the list of items.

[^1]: We still need `prototype.js` in the map widget. See jpatokal#1224.
reedy pushed a commit to reedy/openflights that referenced this issue Jul 10, 2023
This code hasn't seen any action since it was created in early 2010. It
has suffered major bitrot. The state of the code makes it impossible to
actually integrate into any modern website. The fact that the widget
requires running your own instance of OpenFlights is also a major
dealbreaker.

The demo as seen on the actual website is broken in a number of ways:
https://openflights.org/widget/demo

Closes jpatokal#1224.
@jpatokal
Copy link
Owner

Belated comment: yes, this is fine, it's indeed long obsolete.

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 a pull request may close this issue.

2 participants