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 device language getter #211
Conversation
can someone put the steps to replicate the issue so I can try it? |
on this branch, by deleting the property
|
Please ignore this, there isn't a way for a user to do this. The app always has both languages set in local storage.
Open the chrome browser, clear all the localStorage, refresh the app and I see error in console. You need to find a browser that returns the language without
This is expected if your device defaults to English language, so the browser returns english. |
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 fixes it for me on the device.
did you refresh the browser after the second step? |
updated the steps, there was information missing |
@hueitan https://937-212663631-gh.circle-artifacts.com/0/screenshots/experiences.js/Article%20view%20--%20check%20footer.png |
cypress/integration/experiences.js
Outdated
}) | ||
|
||
it('check footer', () => { | ||
articlePage.selectOptionFromActionsMenu('sections') |
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.
I just check on cypress, when you have a new it()
scope, it clears all the local storage item.
Therefore, the following script is needed at the beginning of the scope
cy.setLocalStorage('has-onboard-before', true)
cy.visit('http://127.0.0.1:8080/#/article/en/Cat')
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.
@jpita does this work for you?
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.
well, the has-onboard-before
variable has nothing to do with the language.
shall we ignore this and merge the code?
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.
as per the meeting discussion, please ignore this and merge the code without experience.js.
This reverts commit b5c47fa.
A hotfix to fix the getter function in the previous pr #208
Phab Ticket : https://phabricator.wikimedia.org/T249710#6072863