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

Narrowing with a contextual message id should select that message #954

Closed
neiljp opened this issue Mar 19, 2021 · 5 comments · Fixed by #967
Closed

Narrowing with a contextual message id should select that message #954

neiljp opened this issue Mar 19, 2021 · 5 comments · Fixed by #967
Labels
bug Something isn't working missing feature: user A missing feature for all users, present in another Zulip client

Comments

@neiljp
Copy link
Collaborator

neiljp commented Mar 19, 2021

If using a link such as generated in a quoted message, then the UI doesn't select that message - which seems like the behavior to be expected.

To reproduce:

  • quote a message and send it
  • move cursor onto message with quote
  • open message info popup, move cursor onto link to original message & press enter

I'd expect the original message to be selected, but we don't do this.

Current behavior is:

  • if narrow is loaded/open, do nothing?
  • otherwise, load messages around message id; unclear which message is selected?
@neiljp neiljp added the bug Something isn't working label Mar 19, 2021
@zee-bit
Copy link
Member

zee-bit commented Mar 24, 2021

@neiljp quoting a message generates a near message url (internal link) to the quoted message, which is already handled by #708?
And, I am not able to reproduce as you said. The UI seems to select the message fine for me.
Could you please clarify once?

@neiljp
Copy link
Collaborator Author

neiljp commented Mar 24, 2021

At minimum this doesn't occur when within the same narrow.

I thought it also didn't work for some cases when across narrows, but we may need to check manually or maybe check/add tests for this?

@zee-bit
Copy link
Member

zee-bit commented Mar 24, 2021

I already did a few manual tests for across narrows, and it seemed to work fine for all cases.

I am not sure if adding tests would be any better since we have most of the tests related to parsing/narrowing already implemented in #708(for MessageLinkButton) and generating near message url in test_server_url.py?

@neiljp
Copy link
Collaborator Author

neiljp commented Mar 28, 2021

Have you tried within the same narrow?

zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Mar 29, 2021
This is bugfix that allow selecting of messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.

Earlier, if the user was within the same narrow then narrowing to
the quoted message from MsgLinkButton did not select the message.

Fixes zulip#954.
@zee-bit
Copy link
Member

zee-bit commented Mar 29, 2021

Okay, understood.
So the problem is quoting generates a link to the message in the ['stream', 'topic'] narrow, and if you are already in the same topic narrow, model.set_narrow() returns True (it assumes you are already in the correct narrow), and the message doesn't get selected. For all other cases, it seems to select the message fine.

Solution - Check if contextual msg-id (anchor) is provided to the controller._narrow_to method and don't return early in that case, but shift the focus to select that message. A fix has already been pushed as #967

zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Mar 31, 2021
This is bugfix that allow selecting of messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.

Earlier, if the user was within the same narrow then narrowing to
the quoted message from MsgLinkButton did not select the message.

Tests added.
Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Mar 31, 2021
This is bugfix that allow selecting of messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.

Earlier, if the user was within the same narrow then narrowing to
the quoted message from MsgLinkButton did not select the message.

Tests added.
Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Apr 17, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.

Earlier, if the user was within the same narrow then narrowing to
a quoted message from MsgLinkButton did not select the message.

Tests added.
Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Apr 29, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.
Earlier, if the user was within the same narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case. The
test which was earlier failing, thus indicating the presence of this
bug, is now fixed.

Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue May 2, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.
Earlier, if the user was within the same narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case. The
test which was earlier failing, thus indicating the presence of this
bug, is now fixed.

Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue May 2, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.
Earlier, if the user was within the same narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case. The
test which was earlier failing, thus indicating the presence of this
bug, is now fixed.

Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue May 6, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.
Earlier, if the user was within the same narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case. The
test which was earlier failing, thus indicating the presence of this
bug, is now fixed.

Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue May 21, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.
Earlier, if the user was within the same narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case. The
test which was earlier failing, thus indicating the presence of this
bug, is now fixed.

Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jun 2, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.
Earlier, if the user was within the same narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case. The
test which was earlier failing, thus indicating the presence of this
bug, is now fixed.

Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jun 23, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.

Earlier, if the user was within the same topic narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case.

Fixes zulip#954.
zee-bit added a commit to zee-bit/zulip-terminal that referenced this issue Jun 24, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual msg-id (anchor) is provided to the _narrow_to function.

Earlier, if the user was within the same topic narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case.

Tests amended.

Fixes zulip#954.
neiljp pushed a commit to zee-bit/zulip-terminal that referenced this issue Jun 25, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual message-id (anchor) is provided to the _narrow_to function.

Earlier, if the user was within the same topic narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case.

Tests amended.

Fixes zulip#954.
neiljp pushed a commit that referenced this issue Jun 25, 2021
This is bugfix that allow focusing on messages from the same narrow
if a contextual message-id (anchor) is provided to the _narrow_to function.

Earlier, if the user was within the same topic narrow then narrowing to a
quoted message from MsgLinkButton did not select the message, since the
_narrow_to() function didn't took anchor into account in this case.

Tests amended.

Fixes #954.
@neiljp neiljp added the missing feature: user A missing feature for all users, present in another Zulip client label Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working missing feature: user A missing feature for all users, present in another Zulip client
Projects
None yet
2 participants