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

Unsupported API in notebook - screen.orientation #4875

Closed
akhenry opened this issue Feb 17, 2022 · 4 comments · Fixed by #5172
Closed

Unsupported API in notebook - screen.orientation #4875

akhenry opened this issue Feb 17, 2022 · 4 comments · Fixed by #5172
Labels
bug:platform specific to OS/Browser/Installation method severity:critical type:bug
Projects
Mobile Support Improve Test Coverage Build 6 Blockers
Milestone
Target:2.0.7

Comments

@akhenry
Copy link
Contributor

akhenry commented Feb 17, 2022 •
edited

Summary

The standard screen orientation API is, unsurprisingly, not supported in Safari. Instead our code should feature-detect for screen.orientation, but fall back on the (now deprecated) window.orientation if it's undefined.
@unlikelyzero unlikelyzero added the bug:platform specific to OS/Browser/Installation method label Feb 17, 2022
@unlikelyzero
Copy link
Collaborator

Adding bug:platform to track as platform specific

@scottbell scottbell mentioned this issue Mar 9, 2022
Merged
15 tasks
@scottbell scottbell added this to To do in Mobile Support Mar 9, 2022
@ozyx ozyx self-assigned this May 5, 2022
ozyx added a commit that referenced this issue May 6, 2022
@ozyx
Update Agent.isPortrait() utility method (#4875)
0095d08 
- Prefer to use screen.orientation API first

- Then check for window.orientation (e.g. if using Safari browser on a mobile device)

- Finally, fall back to using window.innerWidth/window.innerHeight
@ozyx ozyx moved this from To do to In progress in Mobile Support May 7, 2022
ozyx added a commit that referenced this issue May 8, 2022
@ozyx
Properly feature detect for orientation APIs (#4875)
5074e05 
- Prefer to use hasOwnProperty() over truthy check
ozyx added a commit that referenced this issue May 8, 2022
@ozyx
Use Agent to detect device orientation (#4875)
9df06ef 
- Provides Agent instance to Notebook component

- Determine device type and orientation using Agent methods instead of checking against body.classList
@ozyx ozyx mentioned this issue May 9, 2022
Merged
15 tasks
ozyx added a commit that referenced this issue May 9, 2022
@ozyx
Tests for display orientation detection (#4875)
cc4cd6c
@ozyx
Copy link
Member

ozyx commented May 9, 2022 •
edited

Test Procedures

Note: Optionally, you can use XCode to simulate mobile Apple device hardware in order to test this. Thanks for the tip @nikhilmandlik !

Notebook no longer throws error if screen.orientation is not available

  • Run a local instance of OpenMCT.
  • Add a Notebook. Add an entry into the notebook.
  • Access the app instance from a mobile device with the Safari browser (e.g.: Plug in iPhone and access it over a shared network)
  • Select the Notebook. Change device orientation a few times from portrait to landscape, and back.
  • View the Safari console on the host machine. Notice that there are no TypeErrors thrown.

Notebook sidebar correctly covers/pushes notebook entries based on device and orientation

  • Run a local instance of OpenMCT.
  • Add a Notebook. Add an entry into the notebook.
  • Open the instance in a browser, such as Google Chrome.
  • Open the "Section/Page" Notebook sidebar. Notice that it does not cover the notebook entries
  • Switch to "Responsive Design Mode". Switch device to a mobile device such as "IPhone 12" and refresh the page
  • Open the "Section/Page" Notebook sidebar. Notice that it now covers the notebook entries.
  • Switch orientation to landscape. Open the sidebar. Notice that it still covers the notebook entries.
  • Switch the device to a tablet, such as an IPad, and refresh the page.
  • In portrait orientation, open the sidebar. Notice that it covers the notebook entries.
  • Switch to landscape orientation, open the sidebar. Notice that it no longer covers the notebook entries

Mobile Support automation moved this from In progress to Done May 20, 2022
akhenry added a commit that referenced this issue May 20, 2022
@ozyx @akhenry
Better detection of device orientation based on browser feature avail…
037886a 
…ability (#5172)

* Update Agent.isPortrait() utility method (#4875)
* Properly feature detect for orientation APIs (#4875)
* Use Agent to detect device orientation (#4875)
* Tests for display orientation detection (#4875)

Co-authored-by: Andrew Henry <akhenry@gmail.com>
@khalidadil
Copy link
Contributor

@ozyx I'm noticing that in iPad portrait orientation, the sidebar doesn't cover the notebook entries (see the image below). Is this right? This is with an iPad Pro 11 inch (3rd generation) running iOS 15.2.

Screen Shot 2022-06-15 at 5 51 41 PM

@khalidadil
Copy link
Contributor

After talking with @ozyx, it sounds like the main bug was addressed for this ticket - so verified fixed on 06/15/22

There was a separate visual bug logged here: #5384

@unlikelyzero unlikelyzero added this to To triage in Improve Test Coverage via automation Jul 17, 2022
@unlikelyzero unlikelyzero added this to Needs triage in Build 6 Blockers via automation Jul 17, 2022
@unlikelyzero unlikelyzero added this to the Sprint:2.0.7 milestone Jul 17, 2022
@unlikelyzero unlikelyzero moved this from Needs triage to Unit Test Added in Build 6 Blockers Jul 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug:platform specific to OS/Browser/Installation method severity:critical type:bug
Projects
Build 6 Blockers
Unit Test Added
Improve Test Coverage
To triage
Mobile Support
Done
Milestone
Target:2.0.7
Development

Successfully merging a pull request may close this issue.

Better detection of device orientation based on browser feature availability nasa/openmct
4 participants
@khalidadil @akhenry @ozyx @unlikelyzero