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

[TIMOB-25646] (7_0_X): iOS After Listview Search overlay not appearing #9731

Merged
merged 10 commits into from Jan 23, 2018

Conversation

vijaysingh-axway
Copy link
Contributor

@build
Copy link
Contributor

build commented Jan 14, 2018

Fails
🚫

🔬 There are library changes, but no changes to the unit tests. That's OK as long as you're refactoring existing code, but will require an admin to merge this PR. Please see README.md#unit-tests for docs on unit testing.

Messages
📖

👍 Hey!, You deleted more code than you added. That's awesome!

📖

💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

Copy link
Collaborator

@hansemannn hansemannn left a comment

Choose a reason for hiding this comment

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

Left a minor note. Also there seems to be a UI-glitching when switching orientations between portrait and landscape as attached here.


- (void)keyboardWillChangeFrame:(NSNotification *)notification
{
NSDictionary *userInfo = [notification userInfo];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unused property. Either remove or change the below statement to use it. Same for keyboardDidChangeFrame and the two implementations for the TiUITableView.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change the statement to use the property.
We are managing the view of SearchController, due to that there is some glitch in orientation change. In slow animation (debug), it is more visible. In normal orientation it will not be visible that much. I'll create a ticket to work on this issue.

@mukherjee2 mukherjee2 self-requested a review January 16, 2018 19:22
Copy link
Contributor

@mukherjee2 mukherjee2 left a comment

Choose a reason for hiding this comment

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

Passed FR with this environment:
Node Version: 8.9.1
NPM Version: 5.5.1
Mac OS: 10.13.1
Appc CLI: 7.0.1
Appc CLI NPM: 4.2.11
Titanium SDK version: 7.0.2 (with artifacts from this PR)
iOS 11.1 Device

Was able to reproduce the issue on the affected version. On fixed version, overlay appeared correctly after Listview search.

@eric34 eric34 merged commit 3572c19 into tidev:7_0_X Jan 23, 2018
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