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.

Sign up for GitHub

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

Jump to bottom

Add warning banner if moving topic or multiple messages to a pre-existing conversation. #31151

Open
laurynmm opened this issue Jul 29, 2024 · 5 comments · May be fixed by #31279 or #32471
Open

Add warning banner if moving topic or multiple messages to a pre-existing conversation. #31151

laurynmm opened this issue Jul 29, 2024 · 5 comments · May be fixed by #31279 or #32471
Assignees
@heyiming
Labels
area: message-editing (topics) area: popovers Popovers, including general message actions. help wanted in progress priority: high

Comments

@laurynmm
Copy link
Collaborator

laurynmm commented Jul 29, 2024 •
edited by alya
Loading

In #28254, we added a confirmation modal if a topic was being renamed to a pre-existing topic via the topic header bar.

We should give a similar warning in:

  • The "Move topic" modal.
  • The "Move messages" modal, when either the "Move this and all following messages in this topic" or "Move all messages in this topic" option is selected.

The warning should be shown when moving messages to any other pre-existing conversation (topic only moves and topic/channel moves).

In particular, the following yellow banner should appear at the top of the modal:

You are moving messages to a topic that already exists. Messages from these topics will be combined.

Notes:

  • The banner should not appear if no changes in the channel or topic name have been made yet (the "Confirm" button will be disabled).
  • The banner should disappear if it no longer applies.
screenshot of move messages modal

Screenshot from 2024-07-29 18-36-15

CZO thread
@alya alya changed the title Add warning banner to move messages modal if moving multiple messages to a pre-existing conversation. Add warning banner if moving topic or multiple messages to a pre-existing conversation. Jul 29, 2024
@heyiming
Copy link
Collaborator

@zulipbot claim

@heyiming
Copy link
Collaborator

heyiming commented Jul 30, 2024 •
edited
Loading

@laurynmm or @alya what is the style/look-and-feel for for "yellow banner"? Is there an example?
Edit: something like "message validation warnings" here?

@alya
Copy link
Contributor

alya commented Jul 31, 2024

Yes, that's right. We sometimes show banners in the invitations modal, in case that's also a helpful reference.

@heyiming
Copy link
Collaborator

heyiming commented Aug 4, 2024 •
edited
Loading

@alya

Screen.Recording.2024-08-04.at.2.43.34.PM.mov
Screen.Recording.2024-08-04.at.2.43.58.PM.mov

This is what I have now. For moving messages, technically "Move all" and "Move this and the following" can move only one message based on the topic or the position in the thread for the selected message. Do we need to worry about this corner case?

@laurynmm
Copy link
Collaborator Author

laurynmm commented Aug 5, 2024

@heyiming - I'd recommend opening a pull request with your changes, and asking questions like this on the linked CZO (chat.zulip.org) thread in the issue description above. It can be hard to understand exactly the corner case you're asking about from looking at screencasts on GitHub, while it can be easier to have an interactive conversation about them on CZO.

One thing I did notice in your screencasts is that the topic name set in the modal for the change, "old wifi isn't compilin", isn't in the left sidebar for the #devel channel. We only want to show these warning banners in the case that the channel/topic where the messages would be moved to isn't empty (i.e., the topic already exists in that channel). If that topic in that channel doesn't exist, then we don't need to show the banner.

heyiming added a commit to heyiming/zulip that referenced this issue Aug 6, 2024
@heyiming
Add warning banner if moving topic or multiple messages to a pre-exis…
d337fb7 
…ting conversation. zulip#31151

Only show the warning banner if submit button is enabled and the number of messages being moved are more than one.
@heyiming heyiming linked a pull request Aug 6, 2024 that will close this issue
Open
12 tasks
heyiming added a commit to heyiming/zulip that referenced this issue Aug 6, 2024
@heyiming
Add warning banner if moving topic or multiple messages to a pre-exis…
e96f452 
…ting conversation. zulip#31151

Only show the warning banner if submit button is enabled and the number of messages being moved are more than one.

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Aug 8, 2024
@heyiming
message move: Add warning moving topic/multiple messages to a pre-exi…
18d49bb 
…sting conversation

Only show the warning banner if submit button is enabled and the number of
 messages being moved are more than one.

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Aug 8, 2024
@heyiming
message move: Update warning banner logic
8478be6 
The warning banner only shows when
1) submit button is NOT disabled, and
2) target topic is an existing topic, and
3) propagate_mode is NOT change_one

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Aug 16, 2024
@heyiming
message move: Add warning moving topic/multiple messages to a pre-exi…
0eede65 
…sting conversation

Only show the warning banner if submit button is enabled and the number of
 messages being moved are more than one.

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Aug 16, 2024
@heyiming
message move: Add warning moving topic/multiple messages to a pre-exi…
de4f17a 
…sting conversation

Only show the warning banner if submit button is enabled and the number of
 messages being moved are more than one.

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Aug 22, 2024
@heyiming
message move: Show warning on moving messages to a conversation.
6c15600 
Only show the warning banner if submit button is enabled and the number of
 messages being moved are more than one.

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Aug 22, 2024
@heyiming
message move: Show warning on moving messages to a conversation.
a9c4b19 
Only show the warning banner if submit button is enabled and the number of
 messages being moved are more than one.

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Sep 14, 2024
@heyiming
message move: Show warning on moving messages to a conversation.
b0b9c33 
Only show the warning banner if submit button is enabled and the number of
 messages being moved are more than one.

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Sep 14, 2024
@heyiming
Message move: Show warning on moving messages to a conversation.
e20c73d 
Only show the warning banner if submit button is enabled and the number of
 messages being moved are more than one.

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Sep 14, 2024
@heyiming
Message move: Show warning on moving messages to a conversation.
f2e0aa9 
Only show the warning banner if submit button is enabled and the number of
 messages being moved are more than one.

Fixes: zulip#31151
heyiming added a commit to heyiming/zulip that referenced this issue Sep 18, 2024
@heyiming
Message move: Show warning on moving messages to a conversation.
55b6ecf 
Only show the warning banner if submit button is enabled and the number of
 messages being moved are more than one.

Fixes: zulip#31151
Joelute added a commit to Joelute/zulip that referenced this issue Nov 25, 2024
@Joelute
topic_move: Add warning banner if moving messages to existing topic.
4264031 
Moving messages is a powerful tool that helps organize conversations
and keep them in place. However, moving messages to an existing topic can
be messy and create a confusing flow of conversation which is difficult
to undo. We should warn users before moving a topic.

Fixes: zulip#31151.

Co-authored-by: heyiming
Joelute added a commit to Joelute/zulip that referenced this issue Nov 25, 2024
@Joelute
topic_move: Add warning banner if moving messages to existing topic.
8670c23 
Moving messages is a powerful tool that helps organize conversations
and keep them in place. However, moving messages to an existing topic can
be messy and create a confusing flow of conversation which is difficult
to undo. We should warn users before moving a topic.

Fixes: zulip#31151.

Co-authored-by: heyiming
@Joelute Joelute linked a pull request Nov 25, 2024 that will close this issue
Open
12 tasks
Joelute added a commit to Joelute/zulip that referenced this issue Dec 6, 2024
@Joelute
topic_move: Add warning banner if moving messages to existing topic.
95e21b1 
Moving messages is a powerful tool that helps organize conversations
and keep them in place. However, moving messages to an existing topic can
be messy and create a confusing flow of conversation which is difficult
to undo. We should warn users before moving a topic.

Fixes: zulip#31151.

Co-authored-by: heyiming
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@heyiming heyiming

Labels
area: message-editing (topics) area: popovers Popovers, including general message actions. help wanted in progress priority: high
Projects
Zulip release priorities
Status: No status
Milestone
No milestone
4 participants
@alya @heyiming @zulipbot @laurynmm