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

chore(ios): prevent modal windows from being swiped down #11057

Merged
merged 20 commits into from Aug 29, 2019

Conversation

vijaysingh-axway
Copy link
Contributor

@build build added this to the 8.2.0 milestone Jul 17, 2019
@build build requested review from a team July 17, 2019 23:44
@build
Copy link
Contributor

build commented Jul 17, 2019

Fails
🚫 Tests have failed, see below for more information.
Warnings
⚠️

Commit 6d31046257196aa751ed98a0f728606d6dae60ab has a message "chore(ios) : Added support for Xcode 11 / iOS 13 dev environment" giving 2 errors:

  • subject may not be empty
  • type may not be empty
⚠️

Commit e5bd9fc9dd2a72dae983bde75502d892f80ab92f has a message "feat(iOS): Removed IS_XCODE_9 constants from code" giving 2 errors:

  • scope must be lower-case
  • subject must not be sentence-case, start-case, pascal-case, upper-case
⚠️

Commit 5ea4b6b32c3175ac45bb40e04e0614163bed8ac7 has a message "feat(android): Optimized TextField/TextArea handling in TableView (#10745)

  • [TIMOB-26862] Android: Modified TextField/TextArea to not flicker or log warnings when changing "text" property while keyboard is shown

  • Revert "[TIMOB-26862] Android: Modified TextField/TextArea to not flicker or log warnings when changing "text" property while keyboard is shown"

This reverts commit 1d8eb8c.

  • [TIMOB-26862] Android: Modified TableView to better handle rows containing TextFields/TextAreas
  • All rows now keep their existing parent row container view.
  • We don't want to re-parent an EditText parent. Causes havoc with virtual keyboard.
  • Simplified TableView getView() recycling code." giving 2 errors:
  • header must not be longer than 72 characters, current length is 74
  • subject must not be sentence-case, start-case, pascal-case, upper-case
Messages
📖

💾 Here's the generated SDK zipfile.

📖 ❌ 1 tests have failed There are 1 tests failing and 472 skipped out of 4367 total tests.
📖

🚨 This PR has one or more commits with warnings/errors for commit messages not matching our configuration. You may want to squash merge this PR and edit the message to match our conventions, or ask the original developer to modify their history.

Tests:

ClassnameNameTimeError
ios.os#hostname() returns Ti.Platform.address value0
Error: expected '' to equal undefined
fail@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/node_modules/should/cjs/should.js:275:23
value@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/node_modules/should/cjs/should.js:356:23
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/os.test.js:123:31
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/ti-mocha.js:4376:41
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/ti-mocha.js:4763:17
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/ti-mocha.js:4840:23
next@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/ti-mocha.js:4688:20
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/ti-mocha.js:4698:15
next@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/ti-mocha.js:4636:30
file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/ti-mocha.js:4665:13
timeslice@file:///Users/build/Library/Developer/CoreSimulator/Devices/A4D657E1-8DBA-4260-B8D2-C42E1A9F52AB/data/Containers/Bundle/Application/39DACA38-9CF4-4866-8FAA-76428DF4192A/mocha.app/ti-mocha.js:5764:29

Generated by 🚫 dangerJS against 60232ed

jquick-axway and others added 8 commits July 18, 2019 15:46
…dev#10745)

* [TIMOB-26862] Android: Modified TextField/TextArea to not flicker or log warnings when changing "text" property while keyboard is shown

* Revert "[TIMOB-26862] Android: Modified TextField/TextArea to not flicker or log warnings when changing "text" property while keyboard is shown"

This reverts commit 1d8eb8c.

* [TIMOB-26862] Android: Modified TableView to better handle rows containing TextFields/TextAreas

- All rows now keep their existing parent row container view.
- We don't want to re-parent an EditText parent. Causes havoc with virtual keyboard.
- Simplified TableView getView() recycling code.
@hansemannn
Copy link
Collaborator

@vijaysingh-axway Can you squash the commits please? Otherwise it's hard to cherry pick

@vijaysingh-axway
Copy link
Contributor Author

@janvennemann Can you review it please?

@@ -46,6 +46,7 @@ - (void)_destroy

- (void)_configure
{
forceModal = YES;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this set to YES here when it's default value is defined as NO?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For iOS 12, by default it is true. So it is set 'YES' in configuration. For iOS 13, it'll take its value from property. Inside

  • (void)viewDidDisappear:(BOOL)animated
    {
    if (isModal && closing) {
    if (isModal && (closing || !forceModal)) {
    [self windowDidClose];
    }
    }
    we are checking this property for firing close event in iOS 13, which should not affect iOS 12 behaviour. So set it YES.
    This has been already tested by Hans and Donovan.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I was just wondering what's the reason behind that. Thanks for the explanation.

@ssjsamir ssjsamir self-requested a review August 29, 2019 14:12
Copy link
Contributor

@ssjsamir ssjsamir left a comment

Choose a reason for hiding this comment

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

FR Passed Modal windows no longer being swiped down. Tested with the two test case from https://jira.appcelerator.org/browse/TIMOB-27169

Test Environment

MacOS Mojave version 10.14.4
Xcode 11 beta 5
Node.js ^8.11.1
iPhone 8 sim (13.0)
"NPM":"4.2.14","CLI":"7.1.1-master.2"

@sgtcoolguy sgtcoolguy merged commit 118bf47 into tidev:master Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants