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

[NEW] Hide Print for App Compatibility #148

Merged
merged 14 commits into from
Jun 8, 2022

Conversation

AlexanderKanakis
Copy link
Collaborator

No description provided.

src/lib/room.js Outdated
Comment on lines 41 to 45
if (isWebView()) {
// Hide Print
store.setState({ hidePrint: true });
parentCall('hidePrint');
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@AlexanderKanakis This is just setting hidePrint flag when the widget is running in the webview. But I think what we want to achieve is when hidePrint is true we want to not show the Save Transcript button. Is that correct @ear-dev?

Copy link

@ear-dev ear-dev Jun 7, 2022

Choose a reason for hiding this comment

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

yes, that is correct @Shailesh351 We should not be setting either hidePrint or showPrint... we should allow the integration script to call the method it wants, and we should check it for True/False, and respond accordingly

src/lib/room.js Outdated Show resolved Hide resolved
@AlexanderKanakis AlexanderKanakis changed the title [NEW] Hide rint for app compatibility [NEW] Hide Print for App Compatibility Jun 7, 2022
@ear-dev
Copy link

ear-dev commented Jun 7, 2022

@AlexanderKanakis please let Shailesh know when it's ready to be reviewed again... thanks

Comment on lines 50 to 52
return transcript && (!store.state.hidePrint || enableTranscriptMobile);
}
return transcript;
return transcript && !store.state.hidePrint;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of this how about just returning false when hidePrint is true?

const { hidePrint } = store.state;

if (hidePrint) {
	return false;
}

Copy link
Collaborator

@Shailesh351 Shailesh351 left a comment

Choose a reason for hiding this comment

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

@AlexanderKanakis Functionality is working fine. Just one thing... for this much change generally there are only 6-10 build file changes, but in this PR there are around 200. Can you make sure it's up to date?

I generally do yarn build on the latest commit for updating build files

@AlexanderKanakis
Copy link
Collaborator Author

@Shailesh351 I did update my build files after committing my latest changes. The problem might be that after I created my branch from develop, it got updated and I had to fix some conflicts locally.

@Shailesh351
Copy link
Collaborator

@AlexanderKanakis You can try building again after resolving merge conflicts

@ear-dev ear-dev merged commit 843a0c0 into develop Jun 8, 2022
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.

3 participants