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

fix: do not show dialog when choosing file #246

Merged
merged 2 commits into from
Jan 4, 2024

Conversation

shaveko
Copy link
Contributor

@shaveko shaveko commented Oct 19, 2023

related to https://oat-sa.atlassian.net/browse/TR-1327

Description

When testTaker selects a file to upload, the dialog is shown that testTaker leaves fullscreen.

The file cannot be uploaded without leaving fullscreen (please keep in mind the difference between fullscreen mode activated by user clicking fullscreen icon in browser menu window and application-initiated fullscreen mode).

The initial intention was to implement the following flow:

  • user clicks file input -> browser leaves fullscreen
  • user selects file/closes the file selection dialog -> browser goes back fullscreen

Unfortunately that implementation of this flow is not possible due to browser security standards because entering fullscreen has to be initiated by some user interaction like click (https://developer.mozilla.org/en-US/docs/Web/API/Element/requestFullScreen). So we need to display dialog again once we are back from file selection to make user click ok button to enter fullscreen.

However we can show the dialog "Test should be taken in fullscreen mode" only when file selection dialog is closed and that is implemented in the PR

How to test

Enable fullscreen security plugin and set it for delivery. Test with file upload interaction

Copy link
Contributor

@jsconan jsconan left a comment

Choose a reason for hiding this comment

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

It works well with Chrome but it has no effect on Firefox (at least on macOS as the fixed issue is not reproduced).

With Safari, we have an intermediate screen asking to leave the FS mode.

In any case, the flow is as expected: when using the file upload, the FS mode is temporarily deactivated.

One comment regarding the commits:

  • One commit containing both the code change and the bundle, this will make it more complicated if a backport is needed.

Review Checklist

  • New code is covered by tests (if applicable)
  • Tests are running successfully (old and new ones) on my local machine (if applicable)
  • New code respects code style rules
  • New code respects best practices
  • New code is not subject to concurrency issues (if applicable)
  • Feature is working correctly on my local machine (if applicable)
  • Acceptance criteria are respected
  • Pull request title and description are meaningful
  • Pull request's target is not master
  • Commits are following conventional commits
  • Commits messages are meaningful
  • Commits are atomic

@github-actions
Copy link

Version

Target Version 3.7.2
Last version 3.7.1

There are 0 BREAKING CHANGE, 0 feature, 2 fixes

@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (6f24fba) 20.00% compared to head (cfd7db1) 20.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             develop     #246   +/-   ##
==========================================
  Coverage      20.00%   20.00%           
  Complexity        53       53           
==========================================
  Files              5        5           
  Lines            175      175           
==========================================
  Hits              35       35           
  Misses           140      140           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@shaveko shaveko merged commit 6632d92 into develop Jan 4, 2024
4 checks passed
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.

None yet

3 participants