ftlabs / screens

A way to distribute websites to multiple displays
12 stars 5 forks source link

Abstracting components for use in all environments #42

Closed JakeChampion closed 8 years ago

JakeChampion commented 8 years ago

An edit of the conversation had in Slack:

Looking at reducing duplication of the carousel code. Could take just the carousel bits and make a module with events for url change so that each use case handles it itself.

But.... perhaps another solution would be to move the entire viewer code into it's own module, which handles the carousel internally and has events for the url change so each use case can handle it.

This has a downside as making the carousel fully integrated with the viewer logic rather than a separate component but has the upside of being much easier to maintain when fixing issues like the ddos issue and will also make carousels work using electron web views.


If the carousel module contained the logic to parse the query parameters, transform the urls and handle the logic for when to move to the next website; only exposing an event-emitter interface (https://nodejs.org/api/events.html#events_events), then it would be decoupled enough from the consuming applications (electron; chrome extension; standard website html + js) that they could have individual features added without it being a maintenance burden.

Suggested packages which can be used in the carousel abstraction: https://www.npmjs.com/package/array-loop + https://www.npmjs.com/package/query-string + https://github.com/ftlabs/screens/blob/master/server/urls.js


It makes sense to decouple the logic as it is being used in multiple environments (Chrome Extension, Web-viewer and, Electron). I think a first step would be to decouple the carousel logic and have that be used by the web viewer, then we can see if the interface engineered works. After that we could decouple the viewer logic and have that be used by the web viewer. Once we are happy that the two decoupled components work as we want them to we can integrate them into the Chrome Extension and Electron application.

In tandem to this work we can start on the auto-updating electron feature, this would ensure that rolling out the new version of electron (with the decoupled viewer and carousel) can be achieved in a swift manner :simple_smile:

I'll create and issue to track this conversation and we can cover more of the ground in the sprint review today.

JakeChampion commented 8 years ago

This feature would help resolve https://github.com/ftlabs/screens/issues/11

AdaRoseCannon commented 8 years ago

https://github.com/ftlabs/screens-carousel https://github.com/ftlabs/screens-viewer

Both of these needs tests written still but the modules are written.