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

Restore iOS support #4671

Merged
merged 1 commit into from
Jan 3, 2019
Merged

Conversation

kollivier
Copy link
Contributor

@kollivier kollivier commented Jan 3, 2019

...as iOS blockers are either resolved or no longer reproduce.

Summary

This change restores Mobile Safari as a supported platform, as #1049 was addressed and the other two blockers don't reproduce (#1046 and #2158).


Contributor Checklist

PR process:

  • PR has the correct target branch and milestone
  • PR has 'needs review' or 'work-in-progress' label
  • If PR is ready for review, a reviewer has been added. (Don't use 'Assignees')
  • If this is an important user-facing change, PR or related issue has a 'changelog' label
  • If this includes an internal dependency change, a link to the diff is provided

Testing:

  • Contributor has fully tested the PR manually
  • If there are any front-end changes, before/after screenshots are included
  • Gherkin stories have been updated
  • Unit tests have been updated

Reviewer Checklist

  • Automated test coverage is satisfactory
  • PR is fully functional
  • PR has been tested for accessibility regressions
  • External dependency files were updated if necessary (yarn and pip)
  • Documentation is updated
  • Contributor is in AUTHORS.rst

@kollivier kollivier added TODO: needs review Waiting for review OS: iOS labels Jan 3, 2019
@radinamatic
Copy link
Member

Dang, I have no iOS device to test this, will have to add it to my shopping list soon... 😕

@kollivier
Copy link
Contributor Author

If you have a Mac and XCode, you can start Kolibri on your Mac and then use the iOS Simulator to start a virtual iOS device and run Safari. That is how I was testing because I didn't have any devices running iOS 9, which is where the bug was triggering.

@radinamatic
Copy link
Member

Ah, good suggestion @kollivier, didn't think of that route...
In that case I'll have to undust my MacMini, it's been dormant for months 😛

@codecov
Copy link

codecov bot commented Jan 3, 2019

Codecov Report

Merging #4671 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #4671   +/-   ##
========================================
  Coverage    51.42%   51.42%           
========================================
  Files          833      833           
  Lines        25088    25088           
  Branches      3303     3303           
========================================
  Hits         12901    12901           
  Misses       11498    11498           
  Partials       689      689
Impacted Files Coverage Δ
...ore/context_processors/custom_context_processor.py 96.55% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46bc533...e74b4fe. Read the comment docs.

@rtibbles
Copy link
Member

rtibbles commented Jan 3, 2019

Code looks good to me, so I'll leave it to testing to discern this PR's worth!

Copy link
Contributor

@indirectlylit indirectlylit left a comment

Choose a reason for hiding this comment

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

thanks - merging in so we can start testing

@kollivier would you mind updating the browserslist file re: #4672

@indirectlylit indirectlylit merged commit e6b58b4 into learningequality:develop Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants