tidev / titanium-sdk

šŸš€ Native iOS and Android Apps with JavaScript
https://titaniumsdk.com/
Other
2.76k stars 1.21k forks source link

iOS: SearchBar in TableView loses result on enter #13246

Open m1ga opened 2 years ago

m1ga commented 2 years ago

I have searched and made sure there are no existing issues for the issue I am filing

Description

When I search in a TableView and press enter (or the keyboard "search" button) it will lose focus and removes the search value from the searchbar so all items are visible again.

const win = Ti.UI.createWindow({title: 'Window'});
var newData = Ti.UI.createTableViewRow({title: 'element 1'})
var newData2 = Ti.UI.createTableViewRow({title: 'row 2'})
var newData3 = Ti.UI.createTableViewRow({title: 'item 3'})
const table = Ti.UI.createTableView({
    search: Ti.UI.createSearchBar(),
    data: [newData, newData2, newData3]
});
win.add(table);
win.open();

Ti 9.3.2.GA keeps the search value Ti 10.0.2.GA, 10.1.1.GA will clear the searchbar and show all items again.

Also notice that the grey "cover" element is placed to high. It is covering the searchbar but you still can type.

https://user-images.githubusercontent.com/4334997/150122156-1eab6504-6f12-451a-a213-15f53d5450d7.mp4

Expected Behavior

Search value should still be in Searchbar and items should still be filtered when clicking enter/the search button.

Actual behavior

Searchbar is empty, TableView shows all results

Reproducible sample

const win = Ti.UI.createWindow({title: 'Window'});
var newData = Ti.UI.createTableViewRow({title: 'element 1'})
var newData2 = Ti.UI.createTableViewRow({title: 'row 2'})
var newData3 = Ti.UI.createTableViewRow({title: 'item 3'})
const table = Ti.UI.createTableView({
    search: Ti.UI.createSearchBar(),
    data: [newData, newData2, newData3]
});
win.add(table);
win.open();

Steps to reproduce

Platform

iOS

SDK version you are using

10.1.1.GA

m1ga commented 2 years ago

This might be the problem: https://github.com/appcelerator/titanium_mobile/blob/master/iphone/Classes/TiUITableView.m#L1586

from https://github.com/appcelerator/titanium_mobile/pull/12906 (also had a backport to 10.0.x). If I remove that line the search result stays as expected.

m1ga commented 2 years ago

The issue with the grey overlay is located in this part: https://github.com/appcelerator/titanium_mobile/blob/master/iphone/Classes/TiUITableView.m#L1445-L1455 If I remove it the dimmingView is placed at the correct place.

ListView works fine, it's just an issue with TableView

caspahouzer commented 2 years ago

Also onClick of search result row the searchbar is blurred and emptied

hansemannn commented 2 years ago

@m1ga Can you attach a video from the 9.x behavior? And a listview replicate of the test case would be great. Looking at the code, it seems intended to clear the value upon "Return".

m1ga commented 2 years ago

@hansemannn I can check tomorrow, it was a client app where the issue was found at first. This PR included the https://github.com/tidev/titanium_mobile/pull/12906 the clear part

caspahouzer commented 2 years ago

I have one

https://user-images.githubusercontent.com/1126641/160489795-72a232c0-632e-403d-9100-e564f3ada785.MOV

m1ga commented 2 years ago

Here is the test app with 9.3.2:

https://user-images.githubusercontent.com/4334997/161002624-5510bff3-8c3a-460b-8a55-2476a55c4b44.mov

When I click enter/submit the search button it will keep the search results. It started with this PR to dismiss the search when you have multiple search bars

hansemannn commented 2 years ago

Okay. I just wonder for what the other ticket was then. Not exactly clear to me so far.

hansemannn commented 2 years ago

I still don't get it, sorry. @caspahouzer mentioned that the search bar is cleared on click, but that seems to be the same as in 9.x.

caspahouzer commented 2 years ago

If you type into search and the press "search" on keyboard, the search value is empties and the search looses focus. to loose focus is okay, but the value should stay in searchbar and the results of the search should also not be resetted

m1ga commented 2 years ago

Exactly: if a user types in something and press "search" on the keyboard to submit the search (yes its updating but people still press the button to finalize it) the search results shouldn't go away and stay.

Astrovic commented 2 years ago

This bug persists even using 11.0.0.RC Currently this fix gets around the bug https://github.com/tidev/titanium_mobile/issues/13246#issuecomment-1017319670

hansemannn commented 2 years ago

Hi there! I am still having some trouble to see the relation between all the linked issues, different test cases and existing fixes. From looking at your comment @Astrovic, this PR should fix the remaining issue, is that correct?

m1ga commented 2 years ago

@hansemannn I'll check that PR too. Thanks for the fix! Have to check the code from https://github.com/tidev/titanium_mobile/pull/12906 too since that part was added for that issue

jasonkneen commented 2 years ago

@hansemannn fix worked and now I'm using 11.0.0.FIX -- hoping this can get in an update soon!

jasonkneen commented 2 years ago

Also line 315 of TiUITableView.m if you set that to NO then no dimming! yay

m1ga commented 2 years ago

Just to have the last info from Slack in here and it won't get lost:

1: This old PR fixed "properly dismiss search controller after editing" when using e.g. two search controllers or this two window setup mentioned here 2: but that introduced the "SearchBar in TableView loses result on enter" error in this issue 3: your PR reverts one of the lines I've mentioned in the issue which fixes 2 but breaks the code from 1 4: the dark overlay is in the wrong position (videos)

so at the moment there is no PR that fixes all cases.

hansemannn commented 2 years ago

Guys, I am not feeling confident to properly solve this, I am sorry. Would someone who actually has all test cases could take over my PR? I can also try to get @janvennemann take a look, if it's not solved by next week.

m1ga commented 2 years ago

I can't help you there besides testing it at the end. The two tests that should pass are:

1. Keep search results after pressing enter

const win = Ti.UI.createWindow({title: 'Window'});
var newData = Ti.UI.createTableViewRow({title: 'element 1'})
var newData2 = Ti.UI.createTableViewRow({title: 'row 2'})
var newData3 = Ti.UI.createTableViewRow({title: 'item 3'})
const table = Ti.UI.createTableView({
    search: Ti.UI.createSearchBar(),
    data: [newData, newData2, newData3]
});
win.add(table);
win.open();

2. properly dismiss search controller after editing (source PR)

const win = Titanium.UI.createWindow({ layout: 'vertical' });
const toggle = Ti.UI.createButton({ top: 80, title: 'Toggle List' })
const search_a = Ti.UI.createSearchBar({ hintText: 'Search A' });
const search_b = Ti.UI.createSearchBar({ hintText: 'Search B' });
const tableView_a = Ti.UI.createTableView({
    data: [ createSection(5, 'A') ],
    search: search_a,
    height: '40%'
});
const tableView_b = Ti.UI.createTableView({
    data: [ createSection(5, 'B') ],
    search: search_b,
    height: '40%',
    visible: false
});

toggle.addEventListener('click', e => {
    tableView_a.visible = tableView_b.visible;
    tableView_b.visible = !tableView_b.visible;

    if (!tableView_a.visible) {
        search_a.blur();
    }
    if (!tableView_b.visible) {
        search_b.blur();
    }
});

win.add([toggle, tableView_a, tableView_b]);
win.open();

function createSection(rows, suffix) {
    const section = Ti.UI.createTableViewSection({
        headerTitle: `Section ${suffix}`
    });

    for (let i = 1; i <= rows; i++) {
        section.add(Ti.UI.createTableViewRow({
            title: `Row #${i}`
        }));
    }

    return section;
}
m1ga commented 1 year ago

Any update on this issue? The two cases in my comment above should work. Summary:

The current "workaround" is to apply the PR if you know that you only have one TableView in a window and want to keep the search results when pressing enter. Merging that PR would introduce the bug from TIMOB-28492 again

jasonkneen commented 1 year ago

This appears to work fine for me now on 12.0.0.RC on iOS

m1ga commented 1 year ago

no, still loses results when you press enter/search (PR is not merged)

https://user-images.githubusercontent.com/4334997/208728106-4ed79e10-028c-4a89-8880-cc31046adffb.mp4

The old PR only fixed part 2 and if you merge this PR it will break part 2 again but fixes part 1 šŸ˜„

jasonkneen commented 1 year ago

odd -- works perfectly for me on device with 12.

m1ga commented 1 year ago

did you patch it yourself again? You've did that to 11 earlier too. The video was just recorded with 12.0.0.RC (no changes)

m1ga commented 1 year ago

@mbender74 maybe you could have a look here. Check https://github.com/tidev/titanium_mobile/issues/13246#issuecomment-1221387587 Those two examples should work.

This old PR - which is in the current SDK - fixed example 2 (multiple tables in one window) but this line breaks example 1 (keep search results). It would be awesome if both parts will work.

This video https://github.com/tidev/titanium_mobile/issues/13246#issuecomment-1359856420 should keep the filtered results when you press "enter"