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

webengine projection #291

Merged
merged 11 commits into from
Aug 21, 2020
Merged

webengine projection #291

merged 11 commits into from
Aug 21, 2020

Conversation

iCollin
Copy link
Collaborator

@iCollin iCollin commented Jul 27, 2020

implement #275

this PR is ready for review!

if (mem.usedJSHeapSize / mem.jsHeapSizeLimit > 0.75) {
for (var app of self.props.webEngineApps) {
if (app.runningAppId && app.style.position === 'absolute') {
bcController.onExitApplication('RESOURCE_CONSTRAINT', app.runningAppId);
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the app is taking up too much memory, should the HMI also "delete" the iframe? Does that happen indirectly after the OnExitApplication message is sent?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will indirectly remove the iframe after OnAppUnregistered is received by the HMI.

if (manifest.category === 'WEB_VIEW') {
style = {
position: 'absolute',
width: '960px',
Copy link
Collaborator

Choose a reason for hiding this comment

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

The size of the web view needs to take into account the dynamic height and width parameters set in the config and used by the scss files. It might be cleaner to define these styles in an scss file and pass a className within the js code instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, I applied these changes here: ab9eaef

@@ -295,6 +297,11 @@ class UIController {
this.listener.send(RpcFactory.UIPerformInteractionResponse(choiceID, appID, msgID))
}
onSystemContext(context, appID) {
if (context === 'MAIN' || context === 'ALERT') {
store.dispatch(appStoreShowWebView());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should you check that the current route is the appstore/webview before dispatching these actions?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, there's an issue here if an app registers as WEB_VIEW and changes templates. That app's iframe would still be shown. I will look into this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In a9d1f48 I added a variable to the state to check whether we were in /web-view and used this in index.js alongside this variable that reflected the current context. This worked fine, but I realized that checking if we are in /web-view is enough and the context doesn't matter after that. So in a1ab7d8 I removed the old state variable related to the context.

render() {
return (
<div className="webView">
<AppHeader backLink="/" menuName="Apps" icon="false"/>
Copy link
Collaborator

@Jack-Byrne Jack-Byrne Jul 29, 2020

Choose a reason for hiding this comment

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

I assume WebengineAppContainer was not included in this component because the iframe needs to be running when not in view?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exactly, the iframe has to be outside the route context so it can run when we are in a menu etc.

@@ -434,6 +434,9 @@ function ui(state = {}, action) {
case "NON-MEDIA":
app.displayLayout = "nonmedia"
break
case "WEB_VIEW":
app.displayLayout = "webview"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think web-view would be more inline with the naming conventions, except nonmedia is kind of ruining my claim.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

updated in 89d551f

If we changed nonmedia we could fix this special case and just use .toLower() for the registerApp call.

@Jack-Byrne
Copy link
Collaborator

FYI style does not persist over ignition cycles for installed projection apps.
Screen Shot 2020-08-04 at 11 11 21 AM

…close all running webengine apps on resource constraing, wait for core to connect to send GetInstalledApps
@iCollin
Copy link
Collaborator Author

iCollin commented Aug 5, 2020

In 51ed7e7 I removed webengine app style from the state completely, since iframe visibility will now be controlled by the web-view route modifying webViewActive in the state.

I also made it so that all web engine apps will be exited when we are running low on memory instead of just projection apps because I was differentiating them by the style.

Lastly I changed when GetInstalledApps is sent to core so that it won't be sent before the HMI is connected to Core.

@iCollin iCollin merged commit 6bb050e into develop Aug 21, 2020
@iCollin iCollin deleted the feature/webengine_projection branch August 21, 2020 17:07
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