Skip to content

Send page reload message to iOS runtime #1107

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Nov 12, 2015
Merged

Send page reload message to iOS runtime #1107

merged 9 commits into from
Nov 12, 2015

Conversation

Fatme
Copy link
Contributor

@Fatme Fatme commented Oct 27, 2015

Should be merged after this telerik/mobile-cli-lib#511

@Fatme Fatme force-pushed the fatme/page-reload branch from fee6ffc to d2ac672 Compare October 27, 2015 08:13
@teobugslayer
Copy link
Contributor

The linter is not happy

teobugslayer
teobugslayer reviewed Oct 27, 2015
View reviewed changes
lib/device-sockets/ios/socket-request-executor.ts
///<reference path="../../.d.ts"/>
"use strict";

import helpers = require("../../common/helpers");
Copy link
Contributor

Choose a reason for hiding this comment

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

ES6 import

@teobugslayer teobugslayer added this to the 1.5.0 milestone Oct 27, 2015
@Fatme Fatme force-pushed the fatme/page-reload branch 2 times, most recently from 94c2c60 to 52f987f Compare October 28, 2015 09:05
ligaz
ligaz reviewed Oct 28, 2015
View reviewed changes
lib/device-sockets/ios/notification.ts
///<reference path="../../.d.ts"/>
"use strict";

export class IOSNotification implements IiOSNotification {
Copy link

Choose a reason for hiding this comment

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

Is there a need for an interface for this class?

Copy link

Choose a reason for hiding this comment

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

This can safely be just a holder for constants, or even exported variables that can be imported via destructing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I use the interface:
https://github.com/NativeScript/nativescript-cli/pull/1107/files#diff-ec63df112e054b09c6c8a97ec07ecb6aR10,
https://github.com/NativeScript/nativescript-cli/pull/1107/files#diff-3ea8e9be6fce663fc286a4462786a297R31

Copy link

Choose a reason for hiding this comment

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

My bad, I miss the part where we use the $projectData.

@Fatme Fatme force-pushed the fatme/page-reload branch 5 times, most recently from 5ee9cfb to 394dead Compare October 29, 2015 07:50
@Fatme Fatme force-pushed the fatme/page-reload branch 4 times, most recently from 4f430ac to c95e3e1 Compare November 6, 2015 07:28
teobugslayer
teobugslayer reviewed Nov 10, 2015
View reviewed changes
lib/services/usb-livesync-service.ts
private $iOSEmulatorServices: Mobile.IiOSSimulatorService,
private $injector: IInjector,
private $iOSNotificationService: IiOSNotificationService,
private $errors: IErrors,
Copy link
Contributor

Choose a reason for hiding this comment

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

not used

@teobugslayer
Copy link
Contributor

Can we speed up a bit Android livesync, if we execute adb only once in AndroidUsbLiveSyncService.beforeLiveSyncAction callback? I mean code similar to

this.device.adb.executeShellCommand(["rm", "-rf", this.$mobileHelper.buildDevicePath(deviceRootPath, "fullsync"),
    this.$mobileHelper.buildDevicePath(deviceRootPath, "sync"),
    this.$mobileHelper.buildDevicePath(deviceRootPath, "removedsync")]).wait();

teobugslayer
teobugslayer reviewed Nov 10, 2015
View reviewed changes
lib/services/usb-livesync-service.ts
iOSSimulatorRelativeToProjectBasePathAction
).wait();
let fastLivesyncFileExtensions = [".css", ".xml"];
let canExecuteFastLiveSync = (filePath: string) => _.contains(fastLivesyncFileExtensions, path.extname(filePath));
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be moved into a dedicated function. The enclosing one is 120 lines long!

@Fatme Fatme force-pushed the fatme/page-reload branch from 9c6e2fe to 74f7fd6 Compare November 11, 2015 10:52
@ns-bot
Copy link

ns-bot commented Nov 11, 2015

Test FAILed.

@Fatme Fatme force-pushed the fatme/page-reload branch from 74f7fd6 to bece803 Compare November 12, 2015 16:35
@ns-bot
Copy link

ns-bot commented Nov 12, 2015

Test PASSed.

1 similar comment
@ns-bot
Copy link

ns-bot commented Nov 12, 2015

Test PASSed.

@Fatme Fatme force-pushed the fatme/page-reload branch from 4449f01 to d744af0 Compare November 12, 2015 17:19
@ns-bot
Copy link

ns-bot commented Nov 12, 2015

Test PASSed.

@rosen-vladimirov
Copy link
Contributor

👍 after green build and passing linux tests

@Fatme Fatme force-pushed the fatme/page-reload branch from d744af0 to 9c3319c Compare November 12, 2015 17:23
@ns-bot
Copy link

ns-bot commented Nov 12, 2015

Test PASSed.

@Fatme Fatme force-pushed the fatme/page-reload branch from 9c3319c to c8df28b Compare November 12, 2015 17:56
@ns-bot
Copy link

ns-bot commented Nov 12, 2015

Test PASSed.

Fatme pushed a commit that referenced this pull request Nov 12, 2015
Merge pull request #1107 from NativeScript/fatme/page-reload
c579ae8 
Send page reload message to iOS runtime
@Fatme Fatme merged commit c579ae8 into master Nov 12, 2015
@Fatme Fatme deleted the fatme/page-reload branch November 12, 2015 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.5.0
Development

Successfully merging this pull request may close these issues.
6 participants
@Fatme @teobugslayer @ns-bot @vchimev @rosen-vladimirov @ligaz