Skip to content
This repository has been archived by the owner on Jun 9, 2022. It is now read-only.

Abstracting components for use in all environments #42

Closed
JakeChampion opened this issue Dec 9, 2015 · 2 comments
Closed

Abstracting components for use in all environments #42

JakeChampion opened this issue Dec 9, 2015 · 2 comments
Assignees
Milestone

Comments

@JakeChampion
Copy link
Contributor

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
Copy link
Contributor Author

This feature would help resolve #11

@JakeChampion JakeChampion added this to the 3 milestone Dec 11, 2015
@AdaRoseCannon
Copy link
Contributor

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

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants