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

Make the iOS app Universal 🌏 #4720

Merged
merged 1 commit into from
Jul 10, 2018
Merged

Make the iOS app Universal 🌏 #4720

merged 1 commit into from
Jul 10, 2018

Conversation

chadyj
Copy link
Contributor

@chadyj chadyj commented Jun 12, 2018 •
edited by mandrigin
Loading

fixes #4662

Summary:

This enables a universal app so Status can be used on an iPad at native resolution.

Testing notes (optional):

All screens and flows should be tested on various sized iPad's.

Steps to test:

  • Install on an iPad

status: ready

@chadyj chadyj requested a review from mandrigin June 12, 2018 12:29
@ghost
Copy link

ghost commented Jun 12, 2018

Hey @chadyj, thanks for making your first pull request in status-react! ❤️

@status-comment-bot
Copy link

status-comment-bot commented Jun 12, 2018 •
edited by ghost
Loading

branch PR-4720:
apk uploaded to https://i.diawi.com/LcHg5R
ipa uploaded to https://i.diawi.com/cPY1XL

@status-comment-bot
Copy link

status-comment-bot commented Jun 12, 2018 •
edited by ghost
Loading

branch PR-4720:
apk uploaded to https://i.diawi.com/GBnXqZ
ipa uploaded to https://i.diawi.com/ComB8r

@mandrigin mandrigin force-pushed the feature/ipad-support-4662 branch from 8d1c5ba to 848affd Compare June 15, 2018 07:54
@mandrigin mandrigin changed the title [WIP] ipad universal support Make the iOS app Universal 🌏 Jun 15, 2018
mandrigin
mandrigin approved these changes Jun 15, 2018
View reviewed changes
Copy link
Contributor

@mandrigin mandrigin left a comment

Choose a reason for hiding this comment

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

LGTM

@mandrigin mandrigin requested a review from rasom June 15, 2018 07:57
@mandrigin
Copy link
Contributor

Not much to review, but it should be tested thoroughly!

@lukaszfryc lukaszfryc self-assigned this Jun 15, 2018
@lukaszfryc
Copy link
Contributor

@annadanchenko could you test it on your iPad? I'll check on iPad simulators and double check on phones.

@status-comment-bot
Copy link

status-comment-bot commented Jun 15, 2018 •
edited by ghost
Loading

branch PR-4720:
apk uploaded to https://i.diawi.com/KjiFN9
ipa uploaded to https://i.diawi.com/Mo1tWF

@mandrigin mandrigin mentioned this pull request Jun 15, 2018
Closed
@mandrigin mandrigin force-pushed the feature/ipad-support-4662 branch from 848affd to 1e03cbf Compare June 18, 2018 11:54
@status-comment-bot
Copy link

status-comment-bot commented Jun 18, 2018 •
edited by ghost
Loading

branch PR-4720:
apk uploaded to https://i.diawi.com/ZtKTMv
ipa uploaded to https://i.diawi.com/Pv8pK6

@mandrigin mandrigin requested a review from flexsurfer June 18, 2018 14:06
@status-comment-bot
Copy link

status-comment-bot commented Jun 28, 2018 •
edited by ghost
Loading

branch PR-4720:
apk uploaded to https://i.diawi.com/MWPiPV
ipa uploaded to https://i.diawi.com/TusE4S

@annadanchenko annadanchenko self-assigned this Jun 28, 2018
@rasom rasom force-pushed the feature/ipad-support-4662 branch from 1e03cbf to 3a4435c Compare June 28, 2018 16:47
@status-comment-bot
Copy link

status-comment-bot commented Jun 28, 2018 •
edited by ghost
Loading

branch PR-4720:
apk uploaded to https://i.diawi.com/PVsSRs
ipa uploaded to https://i.diawi.com/YouLVY

@statustestbot
Copy link

97% of end-end tests have passed

Total executed tests: 33
Failed tests: 1
Passed tests: 32

Failed tests (1)

Click to expand
1. test_messaging_in_different_networks

Wait for ChatMessageInput
Looking for full text: 'test message'

E selenium.common.exceptions.TimeoutException: Message: 'ChatElementByText' is not found on screen, using: 'xpath://*[starts-with(@text,'test message')]/ancestor::android.view.ViewGroup[@content-desc='chat-item']', during '10' seconds

Device sessions:

Passed tests (32)

Click to expand
1. test_sign_transaction_twice
Device sessions:

2. test_swipe_and_delete_1_1_chat
Device sessions:

3. test_one_to_one_chat_messages
Device sessions:

4. test_copy_and_paste_messages
Device sessions:

5. test_send_transaction_from_daap
Device sessions:

6. test_faucet_console_command
Device sessions:

7. test_username_and_profile_picture_in_chats
Device sessions:

8. test_send_eth_from_wallet_sign_now
Device sessions:

9. test_offline_login
Device sessions:

10. test_open_transaction_on_etherscan
Device sessions:

11. test_send_eth_to_request_from_wallet
Device sessions:

12. test_incorrect_password
Device sessions:

13. test_backup_seed_phrase_and_recover_account
Device sessions:

14. test_offline_messaging_1_1_chat
Device sessions:

15. test_contact_profile_view
Device sessions:

16. test_public_chat
Device sessions:

17. test_public_chat_management
Device sessions:

18. test_network_switch
Device sessions:

19. test_delete_1_1_chat
Device sessions:

20. test_wallet_error_messages
Device sessions:

21. test_transaction_send_command_one_to_one_chat
Device sessions:

22. test_network_mismatch_for_send_request_commands
Device sessions:

23. test_browse_link_entering_url_in_dapp_view
Device sessions:

24. test_transaction_send_command_wrong_password
Device sessions:

25. test_resend_message_offline
Device sessions:

26. test_send_eth_to_request_in_one_to_one_chat
Device sessions:

27. test_profile_picture
Device sessions:

28. test_switch_users
Device sessions:

29. test_set_up_wallet
Device sessions:

30. test_send_stt_from_wallet_via_enter_recipient_address
Device sessions:

31. test_qr_code_and_its_value
Device sessions:

32. test_sign_message_from_daap
Device sessions:

@lukaszfryc
Copy link
Contributor

@annadanchenko do you need my help with testing it on simulators?

@lukaszfryc lukaszfryc removed their assignment Jul 2, 2018
@annadanchenko
Copy link

@chadyj @rasom please, restrict only to portrait mode. Currently, it's possible to use landscape mode on iPAd and there is at least one issue with it: grey line to show "Offline"/"Fetching messages" is shown only for part of the screen in landscape mode. There can be more issues, so proposal is to restrict only to portrait mode first.

@chadyj
Copy link
Contributor Author

chadyj commented Jul 2, 2018

@chadyj @rasom please, restrict only to portrait mode. Currently, it's possible to use landscape mode on iPAd and there is at least one issue with it: grey line to show "Offline"/"Fetching messages" is shown only for part of the screen in landscape mode. There can be more issues, so proposal is to restrict only to portrait mode first.

I'd prefer to document any issues and ship with all orientation for anything minor, such as UI quirks. Ipad support is such an improvement over what we have currently that this is a step forward. We can also get user feedback on any new issues and fix those. But if we don't ship landscape I'm afraid it will take a long time to get out the door.

We can even mention in the release notes that this is a beta feature (like all of our features) and we want the community to test.

@chadyj
Copy link
Contributor Author

chadyj commented Jul 2, 2018 •
edited by ghost
Loading

@mandrigin or @rasom Would you be so kind to update https://github.com/status-im/status-react/blob/develop/CHANGELOG.md as part of this PR?

GitHub
status-react - a free (libre) open source, mobile OS for Ethereum

This was referenced Jul 2, 2018
Closed
Closed
Closed
@annadanchenko
Copy link

@chadyj there are 3 ui issues that does not look nice but if user turn to portrait mode then app can be used. So, potentially they can be fixed later and are not blockers:
#5023
#5024

There is ugly ui issue that happened once but I can't repro it yet
#5025

I've tested on real device iPad mini 3, iOs 11.4
also did quick checks on iPhone 7 plus, iPhone 5s and Galaxy S6 to see there are no regressions and screens there look as before.

@asemiankevich
Copy link
Contributor

removed the label so it could be merged. issues will be fixed right after this one is merged

@goranjovic
Copy link
Contributor

@chadyj could you pls resolve the conflict?

@chadyj
Copy link
Contributor Author

chadyj commented Jul 5, 2018

Would be great for @mandrigin or @rasom to take a look.

@rasom rasom force-pushed the feature/ipad-support-4662 branch 2 times, most recently from e0c34c4 to 9e37d7c Compare July 10, 2018 08:07
@status-comment-bot
Copy link

status-comment-bot commented Jul 10, 2018 •
edited by ghost
Loading

branch PR-4720:
apk uploaded to https://i.diawi.com/mvj78S
ipa uploaded to https://i.diawi.com/d8XMnx

@mandrigin @dmitryn
[#4662] Enable iPad support for the Status app.
14398e3 
Use a screen-size independent launch image (a storyboard).

See more at: https://oleb.net/blog/2014/08/replacing-launch-images-with-storyboards/

Signed-off-by: Dmitry Novotochinov <[email protected]>
@dmitryn dmitryn force-pushed the feature/ipad-support-4662 branch from 9e37d7c to 14398e3 Compare July 10, 2018 08:47
@dmitryn dmitryn merged commit 14398e3 into develop Jul 10, 2018
@dmitryn dmitryn deleted the feature/ipad-support-4662 branch July 10, 2018 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mandrigin mandrigin mandrigin approved these changes

@rasom rasom rasom approved these changes

@flexsurfer flexsurfer Awaiting requested review from flexsurfer

Assignees

@chadyj chadyj

@annadanchenko annadanchenko

Labels
Tested - OK
Projects
No open projects
Legacy QA Automation Project
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

iPad Support
10 participants
@chadyj @status-comment-bot @mandrigin @lukaszfryc @statustestbot @annadanchenko @asemiankevich @goranjovic @rasom @dmitryn