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

Focus_Index_Before_Search doesn't work as intended #975

Closed
Rohitth007 opened this issue Apr 1, 2021 · 4 comments · Fixed by #978
Closed

Focus_Index_Before_Search doesn't work as intended #975

Rohitth007 opened this issue Apr 1, 2021 · 4 comments · Fixed by #978
Labels
area: UI General user interface update bug Something isn't working further discussion required Discuss this on #zulip-terminal on chat.zulip.org
Milestone

Comments

@Rohitth007
Copy link
Collaborator

Suppose one is currently focused on the 10th Stream. After "escaping" search it is expected that the focus is back on the 10th Stream but it rather focuses on the stream it was in focus during the search right before escaping.
wrong-focus
This is also related to the crash in #923

@Rohitth007
Copy link
Collaborator Author

@neiljp Also, wanted to ask if this feature is necessary because if you press q when u are at All messages or Middle Column or Right Column there is no way of knowing which was in focus as each of these are different ListWalkers

@Rohitth007
Copy link
Collaborator Author

Rohitth007 commented Apr 1, 2021

@zulipbot add "further discussion required"

@zulipbot
Copy link
Member

zulipbot commented Apr 1, 2021

ERROR: Label "further discussion needed" does not exist and was thus not added to this issue.

@Rohitth007
Copy link
Collaborator Author

@zulipbot add "further discussion required"

@zulipbot zulipbot added the further discussion required Discuss this on #zulip-terminal on chat.zulip.org label Apr 1, 2021
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 1, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by adding a boolean `in_search_mode` which does not
call `get_focus` untill the search is over. Slight reordering was
done in `__init__` to fit the boolean in an ideal place w.r.t.
readability.

Incidentally, the same commit also corrects what the code for
`focus_index_before_search` was intended to do.

Fixes zulip#923 and Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 1, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by adding a boolean `in_search_mode` which does not
call `get_focus` untill the search is over. Slight reordering was
done in `__init__` to fit the boolean in an ideal place w.r.t.
readability.

Incidentally, the same commit also corrects what the code for
`focus_index_before_search` was intended to do.

Fixes zulip#923 and Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 2, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by adding a boolean `in_search_mode` which does not
call `get_focus` untill the search is over. Slight reordering was
done in `__init__` to fit the boolean in an ideal place w.r.t.
readability.

Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Jun 5, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by adding a boolean `in_search_mode` which does not
call `get_focus` untill the search is over. Slight reordering was
done in `__init__` to fit the boolean in an ideal place w.r.t.
readability.

Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Jun 5, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by adding a boolean `in_search_mode` which does not
call `get_focus` untill the search is over. Slight reordering was
done in `__init__` to fit the boolean in an ideal place w.r.t.
readability.

Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Jun 6, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by moving `get_focus` into the SEARCH_STREAM/TOPIC
conditional.

Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Jun 6, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by moving `get_focus` into the SEARCH_STREAM/TOPIC
conditional.

Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Jun 7, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by moving `get_focus` into the SEARCH_STREAM/TOPIC
conditional.

Tests amended. Bad tests (`test_return_to_focus_after_search`)
removed as they pass even though they don't do the right thing.
Current tests fail before this commit.

Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Jun 7, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by moving `get_focus` into the SEARCH_STREAM/TOPIC
conditional.

Tests amended. Bad tests (`test_return_to_focus_after_search`)
removed as they pass even though they don't do the right thing.
Current tests fail before this commit.

Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Jun 9, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by moving `get_focus` into the SEARCH_STREAM/TOPIC
conditional.

Tests amended. Bad tests (`test_return_to_focus_after_search`)
removed as they pass even though they don't do the right thing.
Current tests fail before this commit.

Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Jun 9, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by moving `get_focus` into the SEARCH_STREAM/TOPIC
conditional.

Tests amended. Bad tests (`test_return_to_focus_after_search`)
removed as they pass even though they don't do the right thing.
Current tests fail before this commit.

Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Jun 14, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by moving `get_focus` into the SEARCH_STREAM/TOPIC
conditional.

Tests amended. Bad tests (`test_return_to_focus_after_search`)
removed as they pass even though they don't do the right thing.
Current tests fail before this commit.

Fixes zulip#975
neiljp pushed a commit to Rohitth007/zulip-terminal that referenced this issue Jun 23, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by moving `get_focus` into the SEARCH_STREAM/TOPIC
conditional.

Tests amended. Bad tests (`test_return_to_focus_after_search`)
removed as they pass even though they don't do the right thing.
Current tests fail before this commit.

Fixes zulip#975.
neiljp pushed a commit that referenced this issue Jun 23, 2021
This commit corrects what the code for focus_index_before_search was
intended to do. The focus index was still being updated even in the
search results.

This is fixed by moving `get_focus` into the SEARCH_STREAM/TOPIC
conditional.

Tests amended. Bad tests (`test_return_to_focus_after_search`)
removed as they pass even though they don't do the right thing.
Current tests fail before this commit.

Fixes #975.
@neiljp neiljp added this to the Next Release milestone Jun 23, 2021
@neiljp neiljp added area: UI General user interface update bug Something isn't working labels Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: UI General user interface update bug Something isn't working further discussion required Discuss this on #zulip-terminal on chat.zulip.org
Projects
None yet
3 participants