-
Notifications
You must be signed in to change notification settings - Fork 741
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
py27 ie11 deprecation warnings #10739
py27 ie11 deprecation warnings #10739
Conversation
Build Artifacts
|
c6fdbc5
to
0f41ff4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, issue was incomplete - we need to detect two IE11 scenarios and flag accordingly.
If both conditions are true, I think the "this browser is IE11" message should take priority.
showBanner() { | ||
return this.ie11Deprecated || this.py27Deprecated; | ||
}, | ||
ie11Deprecated() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this got skipped from the design, but we scoped strings to handle two scenarios:
- The current browser you are using is IE11
- Users of this Kolibri are using IE11
At the moment, we are only detecting (2), but the displayed message seems more like (1) is happening to the user.
I have IE11 users on my device, so I see this, in Chrome:
We should add an extra paragraph in the IE11 warning, conditionalized on whether there are detected IE11 users on the device, or whether the current browser is IE11. If the former, we should display this above the current message: https://github.com/learningequality/kolibri/blob/develop/kolibri/core/assets/src/mixins/commonCoreStrings.js#L1297 if the latter, we should display this one: https://github.com/learningequality/kolibri/blob/develop/kolibri/core/assets/src/mixins/commonCoreStrings.js#L1292
Using the new `DeviceAppBarPage` component, we can ensure the banner i is shown across all of the main pages
f020524
to
0affbf0
Compare
<p v-if="py27Deprecated"> | ||
{{ coreString('pythonSupportWillBeDropped') }} | ||
</p> | ||
<p v-if="currentUserOnIE11"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
Implements banner for deprecation warnings in Device for py2.7 and IE11.
Follow-up Tasks
plugin_data
method's scope trying to debug why the Device plugin was failing to enable - I'm wrapping for the day so I'll update this tomorrow~- [ ] Looking to write front-end tests for the banner to put a bow on this ~
plugin_data
from the backend side and it's fairly low-risk code so I'll probably skipReferences
Closes #10482
Reviewer guidance
Use IE11 on your server and view content as a sign-in user (if you've used IE11 on this database before, skip this step)
Go to Device as super admin and see the warning re: IE11
Run the pex using Python 2.7
Go to Device as super admin and see the warning re: Python 2.7
Testing checklist
PR process
Reviewer checklist
yarn
andpip
)