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.

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

Allow full screen external apps (i.e presentation mode) #6688

Merged
merged 3 commits into from
Apr 1, 2022
Merged

Allow full screen external apps (i.e presentation mode) #6688

merged 3 commits into from
Apr 1, 2022

Conversation

diocas
Copy link
Contributor

@diocas diocas commented Mar 29, 2022

This allows for example presentation apps to enter full screen.

@update-docs
Copy link

update-docs bot commented Mar 29, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@dschmidt
Copy link
Member

Should we allow this always for all apps or should the app opt in to that behavior? Do we have a way for external apps to specify such a setting/option?

@diocas
Copy link
Contributor Author

diocas commented Mar 30, 2022 •
edited
Loading

Don't see any reason why we don't want to allow this for any app, since many of them require it.
But I think we should check what needs to be sandboxed, but that's another issue/pr.

kulmann
kulmann suggested changes Apr 1, 2022
View reviewed changes
Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

@diocas Could you run yarn test:unit -u to update the unit test snapshots?

@diocas diocas force-pushed the up_external_fullscreen branch from cc9ff5f to 16e9242 Compare April 1, 2022 14:32
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 1, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@ownclouders
Copy link
Contributor

Results for oCISSharingInternal2 https://drone.owncloud.com/owncloud/web/24352/60/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUISharingInternalUsers-shareWithUsers_feature-L342.png

webUISharingInternalUsers-shareWithUsers_feature-L342.png

💥 The acceptance tests pipeline failed. The build has been cancelled.

@diocas
Copy link
Contributor Author

diocas commented Apr 1, 2022

When I updated, there was another test that got updated as well.. I've removed it, since it was out of the scope of this PR, but I guess it's what's failing now.

@pascalwengerter
Copy link
Contributor

When I updated, there was another test that got updated as well.. I've removed it, since it was out of the scope of this PR, but I guess it's what's failing now.

Acceptance test failure is either random or caused by your branch being out of synch with the acceptance tests on master, I'll merge this branch into a temporary PR and check that

@pascalwengerter pascalwengerter changed the base branch from master to transfer-6688 April 1, 2022 15:21
@pascalwengerter pascalwengerter merged commit 7ad436f into owncloud:transfer-6688 Apr 1, 2022
fschade pushed a commit that referenced this pull request Apr 1, 2022
@diocas @fschade
Allow full screen external apps (i.e presentation mode) (#6688)
65465bf 
* Allow full screen external apps (i.e presentation mode)

* changelog

* Update tests
@diocas diocas deleted the up_external_fullscreen branch April 7, 2022 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kulmann kulmann kulmann requested changes

@pascalwengerter pascalwengerter Awaiting requested review from pascalwengerter

@fschade fschade Awaiting requested review from fschade

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@diocas @dschmidt @ownclouders @pascalwengerter @kulmann