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

liveReload #635

Merged
merged 3 commits into from
Sep 14, 2017
Merged

liveReload #635

merged 3 commits into from
Sep 14, 2017

Conversation

nickbalestra
Copy link
Contributor

@nickbalestra nickbalestra commented Sep 9, 2017

liveReload feature on the registry when in DEV mode

livereload-2

As per #351

@@ -21,7 +27,8 @@ function componentPreview(err, req, res, component, templates) {
dependencies: _.keys(component.dependencies),
href: res.conf.baseUrl,
qs: urlBuilder.queryString(req.query),
templates: templates
templates,
liveReload
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alphabetic

@@ -12,3 +12,4 @@ html
oc.conf.templates = oc.conf.templates || [];
oc.conf.templates = oc.conf.templates.concat(!{JSON.stringify(templates)});
script(src=baseUrl+'oc-client/client.js')
| !{liveReload}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about when this view is used in the UI? Should we put this here (with a localhost url) only if the registry is on local mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm doing that here -> https://github.com/opentable/oc/pull/635/files#diff-e4f8fa94a02abcf0d568f89a410ab48eR16 (if that view is used on the UI when the registry is not on local mode, nothing is printed)

@@ -43,7 +44,9 @@ module.exports = function(dependencies) {
if (!hotReloading) {
logger.warn(strings.messages.cli.HOT_RELOADING_DISABLED);
} else {
cb(components);
cb(components, done => {
liveReloadServer.refresh('/');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you pick this url? /?

Copy link
Contributor Author

@nickbalestra nickbalestra Sep 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refresh method need you pass a string for the file that was changed. In our case for the moment that's not relevant so I'm passing some dummy content / to simply refresh the listening view (in our case the preview).

(In the future we might co-ordinate livereload to work more closely with out browser-client so that we could perform some sort of hot-reloading. For example immagine using this with the chrome-extension tool to just re-render that specific component...)

@nickbalestra
Copy link
Contributor Author

@matteofigus updated/commented

@matteofigus matteofigus merged commit 96f408f into master Sep 14, 2017
@matteofigus matteofigus deleted the liveReload branch September 14, 2017 18:02
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 this pull request may close these issues.

2 participants